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 using < or > in a reference name #761

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

wouterj
Copy link
Contributor

@wouterj wouterj commented Dec 16, 2023

These characters are valid and only the last <...> must be treated as the embedded URL.

This PR removes using the inline lexer inside references, reSt doesn't allow nested inline nodes and the only thing required is getting the embedded URL (if there is any). This reduces a lot of complexity

@wouterj wouterj force-pushed the html-tags-in-reference branch 2 times, most recently from 7d12189 to cc2e28c Compare December 16, 2023 16:25
Copy link
Contributor

@linawolf linawolf left a comment

Choose a reason for hiding this comment

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

There might be issues with false positives here

@wouterj wouterj force-pushed the html-tags-in-reference branch 2 times, most recently from 30fa949 to dc06ee1 Compare December 17, 2023 12:56
@wouterj
Copy link
Contributor Author

wouterj commented Dec 17, 2023

You are correct, this was too fragile. I've now rewritten the code to delegate parsing the embedded URI to the rules/roles that need it and removing it from the global inline lexer. This is the safest and shouldn't break anything, given embedded URIs do not exist on their own.

@wouterj wouterj requested a review from linawolf December 17, 2023 13:00
@linawolf
Copy link
Contributor

I like it, it will also make things easier for us in the php-domain text-roles

@wouterj wouterj force-pushed the html-tags-in-reference branch from dc06ee1 to 9440e98 Compare December 17, 2023 13:12
These characters are valid and only the last `<...>` must be treated as
the embeded URL. This stops using the inline lexer inside references,
reSt doesn't allow nested inline nodes and the only thing required is
getting the embeded URL (if there is any).
@jaapio jaapio force-pushed the html-tags-in-reference branch from 9440e98 to c0e4b1b Compare December 18, 2023 20:12
@jaapio jaapio enabled auto-merge December 18, 2023 20:12
@jaapio jaapio merged commit e647551 into phpDocumentor:main Dec 18, 2023
37 checks passed
@wouterj wouterj deleted the html-tags-in-reference branch December 18, 2023 21:16
linawolf added a commit to TYPO3-Documentation/guides-php-domain that referenced this pull request Dec 20, 2023
With phpDocumentor/guides#761 the textroles
where changed to not rely on the lexer anymore. This makes it necessary
to switch to a regex.
linawolf added a commit to TYPO3-Documentation/guides-php-domain that referenced this pull request Dec 20, 2023
With phpDocumentor/guides#761 the textroles
where changed to not rely on the lexer anymore. This makes it necessary
to switch to a regex.
linawolf added a commit to TYPO3-Documentation/guides-php-domain that referenced this pull request Dec 21, 2023
With phpDocumentor/guides#761 the textroles
where changed to not rely on the lexer anymore. This makes it necessary
to switch to a regex.
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.

3 participants