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

Open
d0ugal opened this Issue Dec 13, 2014 · 29 comments

Comments

Projects
None yet
@d0ugal
Copy link
Member

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

This comment has been minimized.

Copy link
Member

ericholscher commented Dec 14, 2014

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 notifications@github.com
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

This comment has been minimized.

Copy link
Contributor

marcelstoer commented Feb 27, 2016

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

@d0ugal

This comment has been minimized.

Copy link
Member 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

This comment has been minimized.

Copy link
Contributor

marcelstoer commented Feb 27, 2016

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

This comment has been minimized.

Copy link
Member 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

This comment has been minimized.

Copy link

ergonlogic commented May 24, 2016

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

This comment has been minimized.

Copy link
Contributor

marcelstoer commented May 24, 2016

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

@ergonlogic

This comment has been minimized.

Copy link

ergonlogic commented May 24, 2016

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

This comment has been minimized.

Copy link
Contributor

marcelstoer commented May 25, 2016

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

This comment has been minimized.

Copy link

ergonlogic commented May 25, 2016

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

This comment has been minimized.

Copy link
Contributor

marcelstoer commented Jun 8, 2016

@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

This comment has been minimized.

Copy link
Contributor

sahilTakiar commented Jul 3, 2016

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

@virtualtam

This comment has been minimized.

Copy link

virtualtam commented Apr 3, 2018

Hi,

This issue seems to have been fixed (and/or worked around) by #3361 and #3496, I can confirm the ReadTheDocs search module works properly with MkDocs for the Shaarli Documentation (ref. shaarli/Shaarli#1033)

Thanks @d0ugal, @ericholscher and @humitos :)

@humitos

This comment has been minimized.

Copy link
Member

humitos commented Apr 4, 2018

@virtualtam I'm not very into mkdocs nor search, but, does your last comment mean that we need to close this issue or there is something else missing to track here?

(thanks for the update, though!)

@virtualtam

This comment has been minimized.

Copy link

virtualtam commented Apr 4, 2018

@humitos from a user's perspective, the search feature works for documentation generated with MkDocs and hosted on RTD, so I would suggest closing this issue (unless further changes are needed).

@agjohnson agjohnson modified the milestone: Mkdocs Apr 10, 2018

@agjohnson agjohnson removed the Mkdocs label Apr 10, 2018

@davidfischer

This comment has been minimized.

Copy link
Contributor

davidfischer commented Jun 1, 2018

It sounds like this is resolved.

@safwanrahman

This comment has been minimized.

Copy link
Member

safwanrahman commented Aug 7, 2018

I believe this Issue should be open as we are not indexing MkDocs in Elasticsearch index.

@safwanrahman safwanrahman reopened this Aug 7, 2018

@stsewd

This comment has been minimized.

Copy link
Member

stsewd commented Aug 7, 2018

But it uses the default search from mkdocs (I think this issue was about the search in the docs, not in the rtd.org site?)

@safwanrahman

This comment has been minimized.

Copy link
Member

safwanrahman commented Aug 7, 2018

@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

This comment has been minimized.

Copy link
Member

stsewd commented Aug 7, 2018

@safwanrahman maybe this interest you #4205

@humitos

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

ericholscher commented Aug 7, 2018

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

@stale

This comment has been minimized.

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 label Jan 10, 2019

@stsewd

This comment has been minimized.

Copy link
Member

stsewd commented Jan 10, 2019

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

@humitos

This comment has been minimized.

Copy link
Member

humitos commented Jan 10, 2019

This issue is related to #4588

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