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

Use chapter.source_path for partial includes, not chapter.path #1716

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

petersooley
Copy link

Partial includes are found using relative paths, but relies on the logical path variable rather than the actual source_path.

The Chapter::source_path property was introduced to render URLs differently from how they might be on the filesystem. It's a good change that allows preprocessors to do more with the rendered URLs, but that almost always breaks the links preprocessor. This PR switches to using source_path when looking up relative partial paths.

@ehuss
Copy link
Contributor

ehuss commented Jan 3, 2022

Thanks! Can you say more about what situation involves changing the path in such a way that breaks the links preprocessor?

Also, can you add a test for this?

@ehuss ehuss added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Jan 4, 2022
@petersooley
Copy link
Author

I have a preprocessor mdbook-fs-summary that uses filename prefixes and indicators to generate the summary from the filesystem, but I'd like to strip these indicators from the resulting URLs. It generates BookItem::Chapter(Chapter) where path captures the intended URL path and source_path as the location of the file.

Other preprocessors offering similar behavior of wanting to alter the URL without losing access to the underlying filesystem paths would benefit from this. Like for #1258.

As far as tests go, I've added a new test to validate that the Chapter::path can be maintained differently from Chapter::source_path which is ultimately the API I'm inferring with this PR. Turns out this currently fails as there are still places in the overall project that use Chapter::path when it should probably use Chapter::source_path.

I can continue pursuing this to get to a point where this test passes. Please let me know if this is the wrong direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants