Closed bennyslbs closed 7 years ago
Hi, Thank you for the review.
See my comments inline
2016-10-31 18:17 GMT+01:00 Chris Warrick notifications@github.com:
@Kwpolska commented on this pull request.
In v7/navstories/README.md https://github.com/getnikola/plugins/pull/183#pullrequestreview-6481593:
+NAVSTORIES_MAPPING = {
- DEFAULT_LANG: (
Mapping "Toplevel in permalink" to "Visible text"
The order is as listed here, entries not listed here are included in the end,
example (remove initial #):
("b", "Boo"),
("f", "Foo"),
- ), +} +NAVIGATION_LINKS_POST_NAVSTORIES = {
Format just as NAVIGATION_LINKS, but content included after navstories entries
+} +
`` + +To exclude a single story from the navstories meny add the following +metadata
.. hidefromnav: yep` (yep can be anything) to the story.yes or True
I will change to yes, I will also change the code so that no or False gives the same result as if hidefromnav isn't included, or what is the normal behavior for metadata with boolean value?
In v7/navstories/README.md https://github.com/getnikola/plugins/pull/183#pullrequestreview-6481593:
@@ -1,3 +1,34 @@ This very simple plugin throws all the stories to the navigation bar.
-Proof-of-concept of a ConfigPlugin. By request on the mailing list. +Started as a Proof-of-concept of a ConfigPlugin. By request on the mailing list. + +Changed to map the stories to a hierachical structure in the menu
Wait, how deep down does this go? Do you realise you can have only one level of submenus?
Only one sub-level, i didn't mention that in the README, But I can include this: This plugin generates menus and one level of submenus (further sub-levels are mapped to first level submenus). WARNING: Support for submenus is theme-dependent.
In v7/navstories/README.md https://github.com/getnikola/plugins/pull/183#pullrequestreview-6481593:
@@ -1,3 +1,34 @@ This very simple plugin throws all the stories to the navigation bar.
-Proof-of-concept of a ConfigPlugin. By request on the mailing list. +Started as a Proof-of-concept of a ConfigPlugin. By request on the mailing list. + +Changed to map the stories to a hierachical structure in the menu +based on the permalink structure (defaults to the directory structure +of the posts). + +The menu entries inserted by navstories are inserted after entries from
NAVIGATION_LINKS
. +Entries listed inNAVIGATION_LINKS_POST_NAVSTORIES
are inserted after navstories entries. + +Format ofNAVIGATION_LINKS_POST_NAVSTORIES
is identical toNAVIGATION_LINKS
. + +Sorting and display names in menu can be controlled for top-level entries viaNAVSTORIES_CFG
.no example for this
No, it should be NAVSTORIES_MAPPING, and there is an example for that.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/getnikola/plugins/pull/183#pullrequestreview-6481593, or mute the thread https://github.com/notifications/unsubscribe-auth/AKrkZ07GhDqnvCB3jnNZI4hcGAHS6BXBks5q5iKygaJpZM4KlKK7 .
Previous mail was sent twice by an failure, except a small fix, so the last one had this fix: This plugin generates menus and one level of submenus (further sub-levels are mapped to first level submenus).
Br, Benny
Please respond on GitHub, not via e-mail. As for the hidefromnav
variable, no need for no
or False
support (check presence, not value)
I have updated README.md Code checking hidefromdev are checking for presence, so nothing changed (I have also removed my first comment of the dublets).
I have installed v. 7.8.1 and found that default location for stories have been changed to pages, commit df8d5d2 made navstories work well for both pages and stories.
Commit 5667017 lets navstories only add menu entries if initial path of permalink match an entry in NAVSTORIES_PATH.
I think that was all for now.
I have not a public demo site at the moment, I will create one.
p.permalink(lang) When p.permalink(lang) shall be changed to p.permalink(), what then with p.title(lang)?
Usage of TranslatableSetting I have looked into nikola.py and utils.py, but I am not sure I understand how you would like it implemented. I will soon commit a proposal.
p.permalink(lang)
Please use the lang
argument.
TranslatableSetting
You can use those to avoid some checks when it comes to languages in settings.
navstories_paths = utils.TranslatableSetting('navstories_paths', site.config['NAVSTORIES_PATHS'], site.config['TRANSLATIONS'])
You can use navstories_paths(lang)
to get a translation for lang
, or default to the default language. It will follow the behavior of NAVIGATION_LINKS and other built-in translatable settings.
Edit: Just fixed permalink(lang) - now the links don't link to default language :-).
I think the first loop over the configuration variables is the smartest way to accept:
The loop in the top of the lang loop could be skipped, but I think it is easier to read. Edit: I will change usage of TranslateableSetting as you mentioned above.
I had not seen your previous comment - I have just incorporated it, but reading config variables are in a try-excpet to prevent errors if a variable is missed in conf.py.
Example site https://slbs.dk/n/ have been updated.
How does your plugin behave when /nav/index.html
and /nav/last/index.html
are in output?
I am not sure if I understand your question about /nav/index.html
and /nav/last/index.html
are in output?
It ignores all files in the output folder.
I will add text similar to this to the README.md:
Navstories seaches only for pages with permalinks listed in NAVSTORIES_PATHS
.
It skips the path that matches NAVSTORIES_PATHS
. Next level of the permalink is used in the top level menu. If there are further levels it maps to sub-menus.
I don't know if that answers your question?
But how does it react to /stories/nav.rst
and /stories/nav/last.rst
? Where is nav.rst
displayed and how?
That will not be handled good, it will give two entries in the main menu, one with submenu for all entries in nav/*.rst
and one with nav.rst
.
As I see it it is not possible to make the mapping well, but one way could be moving the top level nav.rst
to first element in submenu, e.g.:
A menu like tihis
nav
|- * Top * (nav.rst)
|- Page 1 (nav/1.rst)
|- Page 2 (nav/2.rst)
Last commit 88ac1a8 changes handling of entries as e.g. nav/*.rst
and one with nav.rst
.
I have changed it a bit compared to the idea I have listed above:
nav
|- Top (nav.rst)
|- * Page 1 (nav/1.rst)
|- * Page 2 (nav/2.rst)
|- * * Page 2a (nav/2/a.rst)
The indention '* ' can be controlled via conf.py.
https://slbs.dk/n have been updated to show this.
I don't think I have more changes to the plugin.
I have implemented the changes, should I increment the version number again, or is it only when on master it should be incremented?
Those version numbers don’t matter.
Thank you for contributing to Nikola! :tada:
The hierachical menu is based on the structure of the stories (permalink).
See: https://groups.google.com/forum/#!topic/nikola-discuss/Ry4LpSMY0Bg