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

mkdocs does not fail if in strict mode and if navigation table refers to non-existent file #1604

Closed
charlesreid1 opened this issue Aug 22, 2018 · 10 comments

Comments

@charlesreid1
Copy link

@charlesreid1 charlesreid1 commented Aug 22, 2018

If I add a reference in my table of contents to a non-existent markdown file, and I run mkdocs in strict mode (either by specifying strict: true in mkdocs.yml or by running mkdocs build --strict), I would expect it to fail. However, it does not. Prior versions of mkdocs (0.17) would fail if the table of contents contained a reference to a non-existent file (even without strict mode).

  1. Why would strict mode not check for non-existent files included in the table of contents? It doesn't make sense to leave that out.

  2. Is there any possibility the strit behavior will be changed to be more like 0.17? Or am I stuck with this and should I sink time into writing my own function to enumerate all files and check to see if files listed in the TOC actually exist (which, ahem, mkdocs was doing before just fine)?

@waylan
Copy link
Member

@waylan waylan commented Aug 22, 2018

This was an intentional design decision. Although one I am not crazy about. It was necessary though, to include a few feature requests.

We added a new feature which is to allow the nav to point to external links (#989). As a reminder, if the MkDocs site is hosted in a subdir of a domain, then external files could still be in the same domain. Therefore, a relative link could still be to an external resource (#1373). How are we to know if the nav item is an external link or a misconfigured internal link? More importantly, how is MkDocs to know which nav items should cause strict mode to fail and which should be allowed?

In an early beta release of 1.0 we were generating INFO messages listing every nav item that did not point to an internal resource. This didn't cause a failure in strict mode, but it was hard to miss. However, we received complaints that users were being confused, thinking that they had a problem when the nav items correctly pointed to external resources (#1564). Therefore, the messages were demoted to DEBUG level. If you run your command in --verbose mode, you will see a list of all such nav items.

All of that said, I'm open to considering a PR which is able to reliably differentiate between intentional external links and misconfigured internal links.

@charlesreid1
Copy link
Author

@charlesreid1 charlesreid1 commented Aug 23, 2018

How are we to know if the nav item is an external link or a misconfigured internal link?

Links generally start with http:// or https:// - and in some special cases, ftp:// or ftps:// or some other exotic protocol. This seems like enough to determine if something in the TOC is an external link or not. If it is not an external link, you can check if it is a file and raise an error otherwise. If it is a link, you let it slide.

Am I missing something?

@waylan
Copy link
Member

@waylan waylan commented Aug 23, 2018

Am I missing something?

Yes, as I explained, some users have requested being able to have relative links to resources in the same server which are not part of MkDocs (see #1373). Those links would be indecernable from internal links.

@charlesreid1
Copy link
Author

@charlesreid1 charlesreid1 commented Aug 23, 2018

Those can't be absolute links? This seems like pretty critical functionality to break for one user's request.

@charlesreid1
Copy link
Author

@charlesreid1 charlesreid1 commented Aug 23, 2018

What I mean is, mkdocs failing when it links to a non-existent file (the 0.17 behavior) is what I expect mkdocs to do (in fact, this change from 0.17 to 1.0 completely pulled the rug out from under some CI tests we had implemented). So, to remove expected behavior (that was implemented in a previous version) for a request from a single user is what does not make sense to me.

@facelessuser
Copy link
Contributor

@facelessuser facelessuser commented Aug 23, 2018

Why not have a toggle to turn off strict path checking? Those who want external links to pass can turn off the path checking. Just an idea.

@waylan
Copy link
Member

@waylan waylan commented Aug 23, 2018

All pre 1.0 behavior was always subject to change without notice. That is the nature of beta software. The features we support with 1.0 we have promised to support going forward (at least until 2.0). We never made that promise about any features in the 0.x series.

Regardless, we had this sort of request more than once. I can't seem to find any others right now.

@waylan
Copy link
Member

@waylan waylan commented Aug 23, 2018

A PR which required all relative links to be verified internal links in "strict" mode might be an acceptable solution. Those who want to allow relative external links aren't likely to make use of strict mode anyway. However, outside of strict mode the behavior should stay as it is.

@facelessuser
Copy link
Contributor

@facelessuser facelessuser commented Aug 23, 2018

Failure in strict mode only makes sense. I use strict mode specifically to catch these kind of things.

waylan added a commit to waylan/mkdocs that referenced this issue Aug 28, 2018
Note that prior to 1.0 this was handled as part of the config 
validation. Now that it is moved out of config validation, we no longer 
have the option to collect all wartnings and then raise an error on 
strict mode. We now exist on the first error and never get to any later 
errors.

Closes mkdocs#1604.
@waylan
Copy link
Member

@waylan waylan commented Aug 28, 2018

After looking at a11437e I realized that I had deleted a TODO comment without addressing the todo item. It appears that the reason no warnings are issued for any navigation items was because I forgot to come back to work it out. In any event, I worked through it in #1613. I expect that will satisfy you @charlesreid1 and @facelessuser. Any feedback is welcome.

@waylan waylan closed this in #1613 Aug 29, 2018
waylan added a commit that referenced this issue Aug 29, 2018
While this feels like a feature change/addition, it is addressing a regression and is therefore being treated as a bug-fix.

Note that prior to 1.0 this was handled as part of the config validation. t is now handled after config validation completes. In strict mode, warnings issued in config validation interrupt the build after config validation is complete--causing the build to not complete. Warnings issued during the build (after config validation passes) allow the build to complete and only exit with a failed return code.

Closes #1604.

This also deletes relative_path_ext.py. That file was supposed to be removed some time ago. Not sure how it managed to return.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.