Add index preprocessor #685
Conversation
README.md is a de facto index file in markdown-based documentation. Hence, we respect to README.md and convert it into index.html.
Michael-F-Bryan
left a comment
Michael-F-Bryan
left a comment
There was a problem hiding this comment.
@weihanglo I really like the idea you're proposing here!
We probably want to add an integration test to make sure the README.md is actually rendered to index.html (look at the other tests in tests/rendered_output.rs for inspiration), but other than that I'm really happy with this PR.
src/preprocess/index.rs
Outdated
| fn run(&self, _ctx: &PreprocessorContext, book: &mut Book) -> Result<()> { | ||
| book.for_each_mut(|section: &mut BookItem| { | ||
| if let BookItem::Chapter(ref mut ch) = *section { | ||
| if ch.path.file_name().unwrap_or_default() == "README.md" { |
There was a problem hiding this comment.
Just out of curiosity, do you think it's worth also checking for something like readme.md or README? So using the file stem instead and a case insensitive match.
There was a problem hiding this comment.
Oh. I almost forgot there is a file_stem method can use. Sounds great!
|
I have some concerns with opt-in this feature by default:
Should we need to inspect this issue? Or just leave the decision for users. Let users opt-out the preprocessor or reorganize their book structure manually. We can also check out how gitbook did on this issue. I tried and it seems that if both |
I'd probably emit a warning (if possible) and then not apply the automatic conversion.
I think this is going to happen quite rarely in practice. Most people have their books in a subdirectory instead of the root directory, so the chance of having I'd prefer for this preprocessor to be included by default, otherwise it'll be hard to discover and never get used. |
|
As for file name conflict issue, I decided to convert all README.md and index.md without regarding any pitfall. Just as the same behavior you metioned above, it only emit some warnings to inform users to fix it. |
|
I like the integration tests and the check for matching different variants of This PR looks about ready to merge. If you've got time, would you be able to make a follow-up PR which documents the |
|
@Michael-F-Bryan |
* Add index preprocessor README.md is a de facto index file in markdown-based documentation. Hence, we respect to README.md and convert it into index.html. * Fix warning for unused variables * Update tests for config * Match file stem case-insensitively for IndexPreprocessor * Add tests for IndexPreprocessor * Update book example to fit index preprocessor
This fixes a problem where custom preprocessors were not being registered when running tests. This was caused by the test function rebuilding the preprocessor map. This removes the code that was rebuilding the preprocessors and removing the IndexPreprocessor when running tests. Skipping IndexPreprocessor was added back in rust-lang#741 to fix rust-lang#724 which was caused by rust-lang#685 which added the IndexPreprocessor. Additionally, rust-lang#1986 added running *all* preprocessors. The IndexPreprocessor was removed because in the past the code was testing against the source directly, and the path from `chapter.path` is the converted `index.md` file, and that filename does not exist in the source. This isn't a problem anymore because due to rust-lang#891 it is not reading from the `src` directory. Note that this results in a minor change where the chapter path changes from `README.md` to `index.md` in the output and the `--chapter` option. I think I'm ok with that change, though it would be easy to switch it back if that's an issue.
2 participants
The PR is for
Add new preprocessor to convert README.md to index.md.
Intent
README.md is a de facto index file in markdown-based documentation. Many version control service such as GitHub, Gitlab and Bitbucket render README.md as index files while browsering source code on their web client. Hence, we may respect to README.md and convert it into index.md which would then convert into index.html.
I think this behavior is what people expect nowadays. It would be nice to include this feature.