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

Fix spelling:word role rendering verbatim #189

Merged
merged 1 commit into from
Oct 9, 2022
Merged

Fix spelling:word role rendering verbatim #189

merged 1 commit into from
Oct 9, 2022

Conversation

Rycieos
Copy link
Contributor

@Rycieos Rycieos commented Oct 6, 2022

A line like:

This :spelling:word`foobared` role needs to be fixed.

would be rendered in the output exactly like that, instead of what it should be:

This foobared role needs to be fixed.

Turns out the cause is quite simple: the nodes.Text object from docutils does not hold both the raw text and the "real" text, so it is being passed the raw text, which includes the role name as we see it.

Add a test case to cover this as well.

NOTE: The other (shorter) way to fix this is simply to change this line:

return super().__call__(role, rawtext, text, lineno, inliner,

to

        return super().__call__(role, text, text, lineno, inliner, 

But I think that is more hacky. If you disagree, I can edit this commit.

Fixes #188

@Rycieos
Copy link
Contributor Author

Rycieos commented Oct 6, 2022

Updated commit fixes linting issues as well as adds an entry to history.rst.

This project should have a CONTRIBUTING.md file to mention that requirement.

tests/test_builder.py Outdated Show resolved Hide resolved
docname = env.docname
good_words = text.split()
directive.add_good_words_to_document(env, docname, good_words)
node = nodes.Text(text)
Copy link
Member

Choose a reason for hiding this comment

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

This does look much simpler. Is the real fix that this Text node is initialized differently than when GenericRole creates it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think so. I struggled to understand the route that the original code took to create the final Text node, but it does seem like that is the case.

A line like:
    This :spelling:word`foobared` role needs to be fixed.

would be rendered in the output exactly like that, instead of what it
should be:
    This foobared role needs to be fixed.

Turns out the cause is quite simple: the nodes.Text object from docutils
does not hold both the raw text and the "real" text, so it is being
passed the raw text, which includes the role name as we see it.

Add a test case to cover this as well.

Fixes #188
Copy link
Member

@dhellmann dhellmann left a comment

Choose a reason for hiding this comment

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

Thanks!

@mergify mergify bot merged commit 7372a46 into sphinx-contrib:master Oct 9, 2022
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.

Role :spelling:word: renders as :spelling:word:<word> instead of just <word>
2 participants