Fix issue 18

[datdenkikniet]

See title.

For now, we simply ignore the README file if it is included in the navigation.

Should the user somehow be notified that this happened?

Fixes #18

[begleynk]

I had a think about this, and I can't actually think of a reason why you shouldn't be able to add a navigation entry to your root page. Can you?

Looking at the code a bit it seems that the issue is that the root path is just not included in the list of possible links that we have. This list comes from src/lib.rs:98.

Not quite sure what the most elegant solution is here yet.

[datdenkikniet]

The only real reason I could find is that I can't quite figure out if it's possible to "start" on a navigated page.

By that, I mean that if we start on a page that is present in the navigation, it'd be a little clunky if that page is not selected in the navigation bar/menu. Given that the README is generally used as the index page, which is not selected in the navigation bar, this could occur.

If there's a way to fix that, I agree that including it in the navigation is most definitely a better solution.

[datdenkikniet]

Perhaps this solution is better?

Now, the top-level README is just included in the link tree, and is always sorted to the top of the page (although this doesn't work if there are pages in folders starting with a number, like /002, because for some reason, those are sorted before /.).

It's slightly hacky still, and I seem to have broken the CSS in dark mode, somehow, but it solves the problem I raised above.

[datdenkikniet]

Another issue has appeared: when including all children of a specific path, should the README link be included by default? This would change the default behaviour of doctave, but would be more consistent with the rest of the child-include rules.

Personally I'm not entirely against the idea of adding a filter/setting that prevents this from happening, but it feels like it will be difficult to implement in a way that is nice, design-wise.

[begleynk]

I think my preference is to not include the README as part of the navigation by default. There's already a link to the root (the title of the site on the left) so it feels unnecessary. I also prefer that when you are at the root of the site, there' no highlighted links on the left - indicating that you are in the root.

Now, the top-level README is just included in the link tree, and is always sorted to the top of the page (although this doesn't work if there are pages in folders starting with a number, like /002, because for some reason, those are sorted before /.).

If you don't give the README file a title as part of the frontmatter, it seems the word "README" is just sorted alphanumerically with the rest of the links. At least this is what I see in my testing?

But to take a step back, I think this is probably how I'd see this work the best:

• By default, the root README file should not be included in the side navigation
• If the user adds it in a link rule, we should put the link to whichever position the user indicated

Again, not sure what the cleanest implementation is here.

Finally, apologies for the back-and-forth here! I don't think I was very clear in my initial message about how this feature could be best implemented. I really appreciate you taking the time to tackle this. Let me know if I can help out in any way here. In fact, feel free to send me an email at nik@doctave.com. Would love to exchange a few messages.

[datdenkikniet]

Yeah, I think I misunderstood the usage of the navigation feature a little.

My config looked a little something like

...
navigation:
  - path: docs/
    children: "*"

For some reason I was quite convinced that worked before, but it actually just panics. With the code for this PR it no longer does that and adds a README link (name depending on the front matter of course). However, in this scenario you'd ordinarily not use the custom navigation feature at all, given that there's nothing custom about the navigation generated in this case.

I think the fix is doing what it's supposed to do, now, with the additional look and rewording of what was actually not working.

I'm glad to contribute! I think it's a cool project and found some problems that were solvable relatively easily :) Tbh I'd rather keep the discussion on this issue on github (so that it's also obvious to others how/when it was solved), but I have shot you a message regardless.

[begleynk]

The issue I have with this implementation currently is that it includes the root README by default in the navigation on the left. This is what you see if you do a clean project install with this branch:

$ doctave init
...
$ doctave serve
...

The doctave.yaml looks like this:

$ cat doctave.yaml
---
title: "My Project"

This feels wrong to me. I think the root README link should only show up if the user explicitly adds the root path to the navigation hierachy.

I actually hadn't thought of the case where the user uses the navigation rule that you showed above. My intuition is that it would just take all the child pages in under /docs and sort them alphanumerically?

Also - agreed. Let's keep the discussion here!

[datdenkikniet]

Right! I realized now that I was also talking about that, but had managed to "miss-test" the code in that regard...

I think the final code-change is how it should be:

The root README link is only included if it is explicitly requested through a navigation rule. Consequently, the README link is not included if no navigation rules are specified, or if navigation rules are specified but none of them request the README to be included.

[begleynk]

Looks good now! Thanks again for taking the time to fix this!