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

Add plugin to create crumbs based on metadata #268

Merged
merged 1 commit into from Apr 22, 2018

Conversation

@tbm
Copy link
Contributor

@tbm tbm commented Apr 19, 2018

The pretty_crumbs plugin takes metadata information from the source
file (by default, the crumbs variable but this can be configured) and
uses that information for the breadcrum instead of the file/dirname.

Thanks to Chris Warrick for answering several questions I had as I
worked on this plugin.

@tbm
Copy link
Contributor Author

@tbm tbm commented Apr 19, 2018

This is my first plugin so feedback appreciated.

It seems crumbs.tmpl was renamed to breadcrumbs.tmpl so I think I should change the name of the plugin to pretty_breadcrumbs.

@tbm tbm force-pushed the pretty_crumbs branch from b9dbe8c to 1e6a131 Apr 19, 2018
@tbm
Copy link
Contributor Author

@tbm tbm commented Apr 19, 2018

v2: fixed a pylint error and changed the name to pretty_breadcrumbs

Copy link
Member

@Kwpolska Kwpolska left a comment

Pretty good, but a few minor criticisms.

We could also think about adding this behavior to core’s utils.get_crumbs?

@@ -0,0 +1 @@
__pycache__
Copy link
Member

@Kwpolska Kwpolska Apr 19, 2018

should not be necessary, we have that in a global .gitignore for this repo

Copy link
Contributor Author

@tbm tbm Apr 21, 2018

Ok

.. crumb: This is a test
```

the breadcrumb generated would be `This is a test` instead of `test`.
Copy link
Member

@Kwpolska Kwpolska Apr 19, 2018

how about defaulting to titles? Try crumb first, if it doesn’t exist, use title

Copy link
Contributor Author

@tbm tbm Apr 21, 2018

Titles are usually quite long and I think the filepath (slug) is better than the title if no crumb is defined.

But maybe this should be a config option.


# By default, the pretty_crumbs plugin looks for the metadata tag `crumb`.
# This can be changed with PRETTY_BREADCRUMBS_TAG.
#PRETTY_BREADCRUMBS_TAG = "nav"
Copy link
Member

@Kwpolska Kwpolska Apr 19, 2018

I honestly wouldn’t bother with configurability.

Copy link
Contributor Author

@tbm tbm Apr 21, 2018

I put the config option in because I use nav for historical reasons but it's easy enough to change it to crumb. I can drop the config if you don't think it's useful.

tag = 'crumb'

def pretty_get_crumbs(path, is_file=False, index_folder=None, lang=None):
lang = lang if lang else self.site.default_lang
Copy link
Member

@Kwpolska Kwpolska Apr 19, 2018

use LocaleBorg’s current language in the default case (tons of examples in Nikola core)

Copy link
Contributor Author

@tbm tbm Apr 21, 2018

Ok

else:
file = os.path.normpath(os.path.join(path, link))
if not is_file:
file += "/" + self.site.config['INDEX_FILE']
Copy link
Member

@Kwpolska Kwpolska Apr 19, 2018

Use os.sep instead of "/" for Windows compatibility

Copy link
Contributor Author

@tbm tbm Apr 21, 2018

I'll use os.path.join(). That's even cleaner

The `pretty_crumbs` plugin takes metadata information from the source
file (by default, the `crumbs` variable but this can be configured) and
uses that information for the breadcrum instead of the file/dirname.

Thanks to Chris Warrick for answering several questions I had as I
worked on this plugin.
@tbm tbm force-pushed the pretty_crumbs branch from 1e6a131 to 71acf99 Apr 21, 2018
@tbm
Copy link
Contributor Author

@tbm tbm commented Apr 21, 2018

We could also think about adding this behavior to core’s utils.get_crumbs?

Well, that's up to you. Let me know if you prefer a plugin or a patch against utils.get_crumbs

@tbm
Copy link
Contributor Author

@tbm tbm commented Apr 21, 2018

I pushed a new version. I haven't removed PRETTY_BREADCRUMBS_TAG yet but I'm happy to do that. I'll defer to you on this.

@Kwpolska Kwpolska merged commit dec75f6 into getnikola:master Apr 22, 2018
1 check passed
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Apr 22, 2018

It’s a plugin, so anything goes. As for an upstream patch, I’m not really sure about it. It can live as a plugin.

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

Successfully merging this pull request may close these issues.

None yet

2 participants