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

Allow formatting in the SUMMARY.md file #100

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Conversation

mgeisler
Copy link
Collaborator

This first changes mdbook-xgettext so that formatting is preserved from the SUMMARY.md file.

Next, mdbook-gettext is changed to translate text from the SUMMARY.md file without formatting: this is necessary since mdbook strips formatting before sending the TOC to the preprocessor.

Fixes #66.

@mgeisler
Copy link
Collaborator Author

TODO: I would like to add an end-to-end test of this: something which verifies that what we extract in mdbook-xgettext works with what mdbook-gettext expects.

@mgeisler
Copy link
Collaborator Author

TODO: I would like to add an end-to-end test of this: something which verifies that what we extract in mdbook-xgettext works with what mdbook-gettext expects.

I suppose the unit tests here are good enough — and we'll be testing this soon enough in Comprehensive Rust.

@mgeisler mgeisler requested review from dyoo and djmitche October 21, 2023 12:20
mgeisler added a commit to google/comprehensive-rust that referenced this pull request Oct 22, 2023
These extra `)` were ignored by the `mdbook` parser, but they began showing up in my testing of google/mdbook-i18n-helpers#100.
djmitche pushed a commit to google/comprehensive-rust that referenced this pull request Oct 23, 2023
These extra `)` were ignored by the `mdbook` parser, but they began
showing up in my testing of
google/mdbook-i18n-helpers#100.
Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only took a brief look; hopefully @dyoo can look in more depth?

@mgeisler
Copy link
Collaborator Author

@dyoo, does this look reasonable to you?

fn add_stripped_summary_translations(catalog: &mut Catalog) {
let mut stripped_messages = Vec::new();
for msg in catalog.messages() {
if !msg.source().contains("SUMMARY.md") {
Copy link
Collaborator

@dyoo dyoo Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps ends_with instead of contains?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that would be nice, but we're actually matching against the full list of filenames here: something like src/foo/bar.md:123 src/SUMMARY.md:234.

I could parse this and it would look something like this:

        if !msg
            .source()
            .split_whitespace()
            .filter_map(|source| source.split_once(':'))
            .map(|(path, _)| Path::new(path))
            .any(|path| path.file_name() == Some(OsStr::new("SUMMARY.md")))
        {
            continue;
        }

However, now that I just tried this out, I realize that we cannot make any strong assumptions on where the SUMMARY.md file is: mdbook allows you to configure the source directory.

So the code above is essentially the same as just checking if SUMMARY.md is found anywhere in the string.

Copy link
Collaborator

@dyoo dyoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

i18n-helpers/src/bin/mdbook-gettext.rs Outdated Show resolved Hide resolved
This first changes `mdbook-xgettext` so that formatting is preserved
from the `SUMMARY.md` file.

Next, `mdbook-gettext` is changed to translate text from the
`SUMMARY.md` file without formatting: this is necessary since `mdbook`
strips formatting before sending the TOC to the preprocessor.

Fixes #66.
@mgeisler mgeisler merged commit 6812d57 into main Oct 25, 2023
5 checks passed
@mgeisler mgeisler deleted the summary-formatting branch October 25, 2023 19:14
@mgeisler mgeisler mentioned this pull request Nov 9, 2023
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 this pull request may close these issues.

Lift restriction on having no formatting in SUMMARY.md
3 participants