Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add search option to Coffeescript docs #5058

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

zdenko
Copy link
Collaborator

@zdenko zdenko commented May 7, 2018

This PR adds an offline search feature for docs (preview)
During the doc:site task, search catalogs are build by parsing the content.
To be able to build the hierarchy of the sections, I've added a documentation/structure.coffee which contains the content tree (object).
And, to avoid redundant work (i.e., parsing the markdown files), I use the same object to build the main content and sidebar navigation.

An element of the structure objects can have folowing keys:

  • id <String> required
  • title <String> required
  • href <String> defaults to "#id"
  • html [<String>, ...] a list of documents from documentation/sections
  • className <String> additional class attribute added to the link in the sidebar
  • children [<Structure>,...] a nested structure

The search results are still not 100% perfect. Additional fine-tuning of the search library options are needed to get better results.
Also, some CSS look and feel changes of the new elements are probably needed as well.

@GeoffreyBooth GeoffreyBooth force-pushed the docSearch branch 2 times, most recently from 64ce85c to f8aca2d Compare May 13, 2018 02:10
…ace that initializes the index, so add new global initializeSearch function that it can call; load scripts from https; improve feedback in doc:site; style cleanup
@GeoffreyBooth
Copy link
Collaborator

Wow, there’s so much going on here. This obviously took a lot of effort. Thanks for diving into this!

I’m not sure how I feel about structure.coffee and the new buildBody and buildSidebar functions needed as a result. I feel like we’re moving a lot of logic out of the templates and into code, which makes it harder to reason about the relationship between the final HTML and the original source. That said, there is a lot of duplication between current master’s body.html and sidebar.html. Currently one needs to remember to update both files correctly in order to add a new section, though we add new sections very rarely (new changelog entries don’t count) so this hasn’t been much of a burden.

One issue in this PR is that the current master sidebar has extra links for mobile: the links from the navbar, that are hidden on mobile, are moved into the sidebar. That’s perhaps a good reason to keep the current template-based approach to generating each file. Also in the other search PR, I had put the search input in the sidebar on mobile, since it’s hidden in the navbar on mobile.

Another somewhat fundamental issue is the indexing of the markdown more or less as plain text, rather than parsing it from Markdown into HTML first. This results in ugly backticks in the search results:
image

This was one of my complaints about the Algolia implementation, that I wanted the inline code blocks rendered as such rather than just plain text (though including bare backticks is worse). Is there a reason we don’t render the Markdown into HTML first and then index the HTML?

Personally I think the styling looked better in the other PR. It also had mobile styling. Would you mind if we used that PR’s HTML and styles?

Other issues:

  • The search suggestions don’t close after clicking one of them, or clicking elsewhere.
  • Lots of words that I know are in the docs don’t return any results. “golden”, “snippet”, “stdout”.
  • The search term underlining in the suggestions doesn’t correspond with the query.

I pushed some commits, to try to optimize things and cleanup styles (I try to follow the Coffeelint style rule that you use single quotes for strings unless it’s an interpolated string). But I thought we should discuss before I went any further.

@zdenko
Copy link
Collaborator Author

zdenko commented May 13, 2018

Wow, there’s so much going on here. This obviously took a lot of effort. Thanks for diving into this!

Yep. Clearly, this is not the best approach, but since I had to combine all of the content, I figured it’s best to open PR earlier to discuss changes.
So, I made as little changes as possible to provide working features.

I’m not sure how I feel about structure.coffee and the new buildBody and buildSidebar functions

At first, I had just a simple method in Cakefile in which I parsed the body content, and there was no need to change the templates. With this, I had content and bookmarks in the search index.
But, then I missed the titles which are in the sidebar.html and adding another loop didn’t seem like a smart move.

buildSidebar and buildBody don’t belong in the Cakefile. I’ll rework this and move code back into the templates. At the time, this was the quick and dirty approach to get search working.

I think, structure.coffee, or some form of it, will have to stay, as it’s needed to build the search index “database”. An alternative solution would be HTML scraping and parsing, which, I believe, we don’t need.

This results in ugly backticks in the search results:

Yes, the search result should be well-formed HTML and not MD.

Is there a reason we don’t render the Markdown into HTML first and then index the HTML?

To build a search index, you need text. Parsing MD seems easier than passing HTML thru a bunch of regexes. But, as said, search results should be rendered as HTML.

Personally I think the styling looked better in the other PR. It also had mobile styling. Would you mind if we used that PR’s HTML and styles?

I think I used that one, but I might mess up something 🤦‍♂️
So no, I don’t mind which styling is used.

Other issues:…

Yes, I’m aware of these issues. I’m fixing them.

I pushed some commits, to try to optimize things and cleanup styles … … But I thought we should discuss before I went any further.

Sure, let me finish few things discussed here, and then run another review.

@zdenko
Copy link
Collaborator Author

zdenko commented May 24, 2018

Since changes from #5067 improved the structure of the document, I refactored few things in the last commit.

  • structure.coffee, buildBody and buildSidebar are removed,
  • cherrio is used to parse the content and build the search collection data.
  • The changelog entries are now parsed in the same way as the rest of the document.
  • The HTML attribute data-level is added to the parent <section> tags, and its value is applied to all children. A lower value means a higher order in the search results list.
  • <h2> tags in entries under the "Language Reference" are changed to <h3>.
  • The search result design is simplified. As we have only two levels of the content, I felt there is no need to show titles in a separate column.
before:after:

There are still some tasks on the to-do list:

  • The search engine doesn't provide 100% accurate results, so I'll probably replace it with a better alternative.
  • Mobile version.
  • Design and layout improvements

Copy link

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rebase

@@ -42,6 +42,7 @@
"babel-core": "6.26.3",
"babel-preset-env": "1.6.1",
"babel-preset-minify": "0.4.0",
"cheerio": "^1.0.0-rc.2",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the latest version of this?

This comment was marked as spam.

This comment was marked as spam.

Copy link

@Sohbeteuronet Sohbeteuronet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recently started using IRC again after a very long time. The also have a [URL=“https://www.forumkolik.net/“]forum[/URL] that is very informative. [URL="https://www.nazarsohbet.com/"]mobile chat rooms[/URL] provide a [URL=“https://www.sohbeteuro.net/“]friendly[/URL] and [URL=“https://www.seviyeli.net/“]accepting[/URL]
environment to those that are looking to engage in conversation.
I would recommend giving them a try.

@@ -42,6 +42,7 @@
"babel-core": "6.26.3",
"babel-preset-env": "1.6.1",
"babel-preset-minify": "0.4.0",
"cheerio": "^1.0.0-rc.2",

This comment was marked as spam.

@@ -42,6 +42,7 @@
"babel-core": "6.26.3",
"babel-preset-env": "1.6.1",
"babel-preset-minify": "0.4.0",
"cheerio": "^1.0.0-rc.2",

This comment was marked as spam.

@Sohbeteuronet

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants