Fix parsing of punctuation in format_links() #3027
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows to parse `(URL).` correctly, which was not detected as URL before.
(Test case already part of test_format_links_simple_url.)
Hiding previous comment about dato#15, which was closed and fast-forwarded hereI also noticed that multiple punctuations signs are not supported. This is a bit trickier to implement, because the current approach asumes only one character at either side is to be stripped. Can be easily solved with a function that, rather than return booleans, splits the string into a prefix/link/suffix triplet. I'll submit that as a separate PR if it doesn't seem too much to review. Thanks! (Since it's a chained PR, it's at dato#15 at the moment. You can fast-forward to this PR if you wish to merge/review all together.) |
dato
changed the title
Also, consolidate all punctuation tests into a single table-driven one.
They're identical here, since re.M is not used, and the better-known should be used, for readability.
dato
force-pushed
the
find_links_wrapped_punct
branch
from
d4d0a7a
to
afad39b
Compare
|
This is really niiiice! Well done! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main fix is not parsing punctuation inside brackets if it's not preceded by a URL. This fixes #2993, fixes #3049. Additionally, multiple punctuation signs are now supported.
As noted in a comment below, the inner workings of the function are changed, making helper functions perform the stripping of symbols, rather than just returning bool.