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

ReadTheDocs search and MkDocs #1088

Closed
d0ugal opened this issue Dec 13, 2014 · 34 comments · Fixed by #6937 or #6986
Closed

ReadTheDocs search and MkDocs #1088

d0ugal opened this issue Dec 13, 2014 · 34 comments · Fixed by #6937 or #6986
Assignees
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code
Milestone

Comments

@d0ugal
Copy link
Contributor

d0ugal commented Dec 13, 2014

(Sorry for adding this as an issue @ericholscher, but we keep missing each other on IRC. If there is a better place, let me know.)

We added the mkdocs json command to MkDocs for RTD's to use in serverside search. Since then I have been working on clientside search support. This means we generate a JSON index in a slightly different format to work with Tipue search and thus we will potentially have two different JSON output formats which is a bit of a pain.

The Tipue format looks like this: https://github.com/Tipue/Tipue-Search/blob/master/tipuesearch/tipuesearch_content.js . The key difference is that it is one file containing all pages and the key names are different but otherwise I think it contains pretty much the same data.

So, my question is, can RTD be adapted to use this format so we can drop the specific json command from MkDocs? If so, I'd be happy to make the change in the ReadTheDocs code.

@ericholscher
Copy link
Member

Seems reasonable. Our code for that is here.

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/search/utils.py#L15

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/projects/tasks.py#L408-L417

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/projects/tasks.py#L656-L657

Not super great logic/code, mostly left over from how we do Sphinx.
Definitely happy to have someone else write it :)

On Sat, Dec 13, 2014 at 8:05 AM, Dougal Matthews [email protected]
wrote:

(Sorry for adding this as an issue @ericholscher
https://github.com/ericholscher, but we keep missing each other on IRC.
If there is a better place, let me know.)

We added the mkdocs json command to MkDocs for RTD's to use in serverside
search. Since then I have been working on clientside search
mkdocs/mkdocs#222 support. This means we
generate a JSON index in a slightly different format to work with Tipue
search and thus we will potentially have two different JSON output formats
which is a bit of a pain.

The Tipue format looks like this:
https://github.com/Tipue/Tipue-Search/blob/master/tipuesearch/tipuesearch_content.js
. The key difference is that it is one file containing all pages and the
key names are different but otherwise I think it contains pretty much the
same data.

So, my question is, can RTD be adapted to use this format so we can drop
the specific json command from MkDocs? If so, I'd be happy to make the
change in the ReadTheDocs code.


Reply to this email directly or view it on GitHub
#1088.

Eric Holscher
Maker of the internet residing in Portland, Or
http://ericholscher.com

@marcelstoer
Copy link
Contributor

Is this still relevant? mkdocs json has since become deprecated...

@d0ugal
Copy link
Contributor Author

d0ugal commented Feb 27, 2016

Yes, I believe this is relevant. We have deprecated it in MkDocs but we won't remove it while RTD still depends on it.

@marcelstoer
Copy link
Contributor

Fair enough. Sorry, I was mislead but I'm grasping at straws trying to figure out why the search for some MkDocs projects works on RTD but not for others (#2020).

@d0ugal
Copy link
Contributor Author

d0ugal commented Feb 28, 2016

No problem. I'm not sure how to help you with that issue, I'm not familiar with RTD's search.

@ergonlogic
Copy link

It appears that the form action for the searchbox is being injected with this javascript: https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/core/static-src/core/js/doc-embed/mkdocs.js.

It looks like it's just building the wrong query, along the lines of: https://readthedocs.org/search/?q=bar&check_keywords=yes&area=default&project=foo&version=X.y&type=file. Either of these paths actually work though:

@marcelstoer
Copy link
Contributor

Thanks for the hint. The first doesn't work for me but the second does:

@ergonlogic
Copy link

The easiest work-around for this is overriding the searchbox template like so: aegir-project/documentation@2884f80...3a44eec

Note the change in the form's id (e.g. rtd-search-form-alt). This stops the RTD JS from breaking the default search functionality.

@marcelstoer
Copy link
Contributor

The fact that your work-around has the desired affect means that RTD by default replaces the MkDocs RTD theme? The original theme's searchbox.html isn't so different from yours: https://github.com/mkdocs/mkdocs/blob/master/mkdocs/themes/readthedocs/searchbox.html. So, the custom theme kinda re-introduces the original theme?

@ergonlogic
Copy link

RTD isn't replacing the mkdocs theme, just altering it with some javascript. As per http://www.mkdocs.org/user-guide/custom-themes/#creating-a-custom-theme, I just copied the default mkdocs RTD theme's searchbox.html, and altered the form id, so that the JS would no longer have any effect.

@marcelstoer
Copy link
Contributor

@ergonlogic the only "problem" with your nifty custom theme approach I found so far is that RTD changes the implementation of the footer fly-out menu it adds. While it normally sits in the lower left corner it is moved to the lower right and becomes detached once you declare a custom theme in mkdocs.yml.
An alternative is to add the following JavaScript function to your extra.js. It reverts the changes RTD applies to the search form:

  function fixSearch() {
    var target = document.getElementById('rtd-search-form');
    var config = {attributes: true, childList: true};

    var observer = new MutationObserver(function(mutations) {
      // if it isn't disconnected it'll loop infinitely because the observed element is modified
      observer.disconnect();
      var form = $('#rtd-search-form');
      form.empty();
      form.attr('action', 'https://' + window.location.hostname + '/en/' + determineSelectedBranch() + '/search.html');
      $('<input>').attr({
        type: "text",
        name: "q",
        placeholder: "Search docs"
      }).appendTo(form);
    });
    // don't run this outside RTD hosting
    if (window.location.origin.indexOf('readthedocs') > -1) {
      observer.observe(target, config);
    }
  }

@sahilTakiar
Copy link

Thanks you @marcelstoer the work-around you implemented in nodemcu/nodemcu-firmware@7dd89dd is working for me!

@safwanrahman
Copy link
Member

@stsewd Yeah. currently it uses built in search of mkdocs. But it would be better if we could index the content to our elasticsearch index so we can provide better search result to our users.

@stsewd
Copy link
Member

stsewd commented Aug 7, 2018

@safwanrahman maybe this interest you #4205

@humitos
Copy link
Member

humitos commented Aug 7, 2018

But it would be better if we could index the content to our elasticsearch index so we can provide better search result to our users

IIRC, the problem we have with MkDocs is there is no way to generate a context to create the .json file as we do with Sphinx. The limitation was on MkDocs side, not on us.

I may be wrong, though.

@safwanrahman
Copy link
Member

safwanrahman commented Aug 7, 2018

@humitos The json file is created with the HTML file by default. But the structure is different. We need to port our code to index the json file into our Elasticsearch.

@ericholscher
Copy link
Member

We need a way to dump mkdocs HTML unthemed, which is the main problem. It needs to be consistent across themes and builds.

@stale
Copy link

stale bot commented Jan 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status: stale Issue will be considered inactive soon label Jan 10, 2019
@stsewd
Copy link
Member

stsewd commented Jan 10, 2019

This is still valid, but blocked and/or needs a design decision

@stale stale bot removed the Status: stale Issue will be considered inactive soon label Jan 10, 2019
@stsewd stsewd added Needed: design decision A core team decision is required Status: blocked Issue is blocked on another issue labels Jan 10, 2019
@humitos
Copy link
Member

humitos commented Jan 10, 2019

This issue is related to #4588

@stsewd stsewd added Accepted Accepted issue on our roadmap and removed Needed: design decision A core team decision is required Status: blocked Issue is blocked on another issue labels Apr 21, 2020
@stsewd
Copy link
Member

stsewd commented Apr 29, 2020

This isn't fully closed. We are going to be indexing search data for mkdocs since next week, but not yet overriding the mkdocs search for ours.

@stsewd
Copy link
Member

stsewd commented Jun 4, 2020

Hopefully this is going live next week, if someone wants to try the server side search for mkdocs projects next week, let us know via email at [email protected].

@marcelstoer
Copy link
Contributor

What are the changes MkDocs-based projects have to make to be compliant with what goes live next week? Or maybe, which work-arounds have to/can be removed next week?

@stsewd
Copy link
Member

stsewd commented Jun 4, 2020

Let me explain this a little more. We used to index search data only for sphinx projects, but now we index search data for mkdocs too (search works here now https://readthedocs.org/projects/mkdocs-clone/search/?q=test&type=file&project=mkdocs-clone&version=latest for example).

With the other change (to be released next week), now you can get those results inside the doc pages. It should work with all mkdocs themes that have the #mkdocs-search-results and mkdocs-search-query elements. For now, you'll need to trigger a re-build for all versions you want search enabled after we enable the feature for your project. After the beta, you should be able to just use the server side search in all projects without needing to re-build

@stsewd
Copy link
Member

stsewd commented Jun 9, 2020

If someone is curious, this is how it looks

If you want to test this let us know via email at [email protected].

rfay added a commit to ddev/ddev that referenced this issue Oct 16, 2021
* Update requirements.txt for mkdocs changes
* Include jquery before other js
* fix-search.js doesn't seem to be needed any more, see readthedocs/readthedocs.org#1088
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code
Projects
None yet