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

If SUMMARY.md doesn't contain a file, mdbook doesn't give an error #540

Closed
est31 opened this issue Jan 13, 2018 · 6 comments · Fixed by #541
Closed

If SUMMARY.md doesn't contain a file, mdbook doesn't give an error #540

est31 opened this issue Jan 13, 2018 · 6 comments · Fixed by #541

Comments

@est31
Copy link
Member

est31 commented Jan 13, 2018

Suppose you changed a line in SUMMARY.md of the example book from:

    - [watch](cli/watch.md)

to

    - [watch](cli/wa_tch.md)

The file wa_tch.md doesn't exist. If you ask mdbook to create the book, it doesn't give you any error or complaint that the file couldn't be found. Instead it generates an empty page. This has led to an issue with Rust's documentation staying undiscovered: rust-lang/rust#47394 .
Therefore mdbook should fail generation of the book if it encounters a file it can't find.

@Michael-F-Bryan
Copy link
Contributor

Michael-F-Bryan commented Jan 13, 2018

This behaviour is actually configurable by adding a build.create-missing item to your book.toml, with the option defaulting to true (check the configuration page in the user guide for more).

It's kinda hard to figure out a sane default here because it's also a super useful feature when you are initially prototyping a book. You can just write out your SUMMARY.md with mdbook watch running in the background and it'll generate everything for you. That said, if you don't know about the feature then I guess it can feel quite magical.

I remember having a discussion with @azerupi over whether we should keep it or not, and he had a pretty compelling argument about why create-missing is a good thing to have.

@steveklabnik
Copy link
Member

steveklabnik commented Jan 13, 2018 via email

@est31
Copy link
Member Author

est31 commented Jan 13, 2018

Interesting @Michael-F-Bryan. I see how this feature can be useful for prototyping. I guess if you both want it kept I guess we shouldn't change defaults any more... I don't use this tool at all so you know best what is most convenient :).

It would be great if Rust's CI could somehow enable this strict mode however to be able to detect bugs earlier. I see two ways:

  • Setting build.create-missing in the book.toml files of all books in the repo.
  • adding a command line flag to mdbook that enables the feature, ignoring what's in book.toml. Then Rust's bootstrap system could pass that flag to all invocations of mdbook.

The first option would mean that you'd always have to flip the option when you want the prototyping mode. It would also make it harder to keep track whether the feature is enabled for all affected books... someone would have to check that this flag is set for new books and if someone flips it locally for better development but doesnt flip it back it might slip under the radar. Maybe a tidy lint could do it but generally I think I prefer the second option.

@steveklabnik how does the doc team commonly invoke mdbook for the books? Do you use x.py? Or do you rather use mdbook directly?

If you use x.py there might be a disadvantage of the second option, as disabling the flag locally is harder (you need to patch bootstrap). But I guess I'm overthinking this issue a bit :p.

@steveklabnik
Copy link
Member

We never create new books via x.py. I think a flag that errors in this situation makes a lot of sense.

@Michael-F-Bryan
Copy link
Contributor

A third option would be to enable it via an environment variable. That should help with both the CI case and if you use x.py (or some other tool).

I believe there's even an old PR which allowed you to override configuration using environment variables, but the config system back then wasn't nearly as easy to use so the PR never got finished.

@est31
Copy link
Member Author

est31 commented Jan 13, 2018

Env var or command line flag, both would be fine.

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 a pull request may close this issue.

3 participants