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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search plugin: added option to put full nav path to the search result title #2590

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

lord-vesel
Copy link

@lord-vesel lord-vesel commented Sep 30, 2021

Hi! Here is my first PR 馃榿

Added option full_path_in_title which allows to get not just page title in a search result, but full path separated with slashes, like: "Section 1 / Subsection / Page Title".

To enable the feature, one has to add following to mkdocs.yml:

plugins:
    - search:
        full_path_in_title: yes

Tests for search plugin is updated too.

@oprypin
Copy link
Contributor

oprypin commented Oct 4, 2021

Hi. Thanks for the contribution. Sorry that I haven't looked it in detail yet.

Basically the main difficulty for a reviewer is... it's hard to decide whether this feature would have wide enough appeal to be "worth it".

The code seems to be high-quality at a glance, though.

I'll think about it more.


Here is my first PR

Something to keep in mind is that usually for any big open source project, the first thing you open should be an Issue. It's a better place to have an initial discussion, and an "opportunity" to get a rejection before putting work into an implementation. Some projects even require an issue to be opened, because there you can discuss the merits of the idea, and then discuss only implementation details in the pull request.

@oprypin
Copy link
Contributor

oprypin commented Oct 4, 2021

You may also want to double-check that the email address you sign the commit with matches an email address in your GitHub account. As it is now, the commit wouldn't be attributed to your GitHub profile.

@oprypin
Copy link
Contributor

oprypin commented Oct 10, 2021

It seems to be working wrong

Before:
image

After:
image

diff --git a/mkdocs.yml b/mkdocs.yml
index bfaeb5588..32fd9f5a4 100644
--- a/mkdocs.yml
+++ b/mkdocs.yml
@@ -7,7 +7,7 @@ repo_url: https://github.com/mkdocs/mkdocs/
 edit_uri: ""
 
 theme:
-    name: mkdocs
+    name: material
     locale: en
     analytics: {gtag: 'G-274394082'}
     highlightjs: true
@@ -53,7 +53,8 @@ markdown_extensions:
 copyright: Copyright &copy; 2014 <a href="https://twitter.com/_tomchristie">Tom Christie</a>, Maintained by the <a href="/about/release-notes/#maintenance-team">MkDocs Team</a>.
 
 plugins:
-    - search
+    - search:
+        full_path_in_title: true
     - redirects:
         redirect_maps:
             user-guide/plugins.md: dev-guide/plugins.md

@lord-vesel
Copy link
Author

It seems to be working wrong

Thanks, and I think I fixed it, now it doesn't mess up internal section titles, modifying only the first title for section.
2021-10-10-19-39-21

page_ancestors = []
full_title = ''

if len(page.ancestors):
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when the page has no ancestors? Then the title is completely empty, and I think that's a problem. Or actually, I'm not noticing anything bad happening, but then why doesn't an empty title cause problems?

@oprypin
Copy link
Contributor

oprypin commented Oct 10, 2021

Agh I don't understand how it works.

It seems that the search can lead you directly to an #anchor and then this change doesn't affect that, but it does affect when it sends to a page in general.

image

Shouldn't it be "User Guide / Configuration / nav" or something

@oprypin
Copy link
Contributor

oprypin commented Oct 10, 2021

Note that I pushed my commit to your branch

@iagodvsantos
Copy link

Any plans to merge this PR? It would be useful in our context

@mkdocs mkdocs deleted a comment from AmandaSatin Apr 22, 2022
@ossdhaval
Copy link

+1.
This feature would be useful to us too!! Path adds context to the search results, and that can help users decide which of the search result is the best fit.

@chuckwagoncomputing
Copy link

I had this idea myself, and here I find someone else had it before me! Please merge this. It would be super helpful for us, as we have headers that are identical to headers on other pages, and it's hard to figure out which page the search result is for.

@rusefillc
Copy link

@lord-vesel any chance to rebase?

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

Successfully merging this pull request may close these issues.

None yet

7 participants