Skip to content

Fix for bad id whitespace collapsing in references #743

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

Merged
merged 3 commits into from
Oct 30, 2018
Merged

Fix for bad id whitespace collapsing in references #743

merged 3 commits into from
Oct 30, 2018

Conversation

facelessuser
Copy link
Collaborator

This fixes #742 by generally collapsing all whitespace down to a single space in reference ids opposed to the previous logic of collapsing only newlines preceeded by whitespace.

This fixes #742 by generally collapsing all whitespace down to a single space in reference ids opposed to the previous logic of collapsing only newlines preceeded by whitespace.
@facelessuser facelessuser added the needs-review Needs to be reviewed and/or approved. label Oct 27, 2018
@vincentbernat
Copy link

Works fine for me and it doesn't change anything on my existing set of markdown documents where I avoided to run into this case. Thanks!

@waylan
Copy link
Member

waylan commented Oct 28, 2018

I'm thinking we need another test. I doubt this would be a problem, but lets make sure this doesn't break things were we shouldn't get a link. For example, a blank line between the two lines:

I would like to tell you about the [code of

conduct][] we are using in this project.

That specific example shouldn't be a problem as the two halves are in different paragraphs (and therefore not in the same block of text), but it illustrates my concern: If the new regex was run against that block of text, it would strip out the blank line in addition to any other extra whitespace. And that would be an error.

I'm just not coming up with an example that might actually occur. I want to make sure we have given it some thought and, at the very least, confirm that the above example works as expected.

@facelessuser
Copy link
Collaborator Author

I imagine the stated example will work as expected. I'm happy to put it in.

I'm also having difficulty coming up with something desired that this could break. Currently, no existing tests fail, so that is a plus. This may be something, that in time, someone will have a contrived example we must account for, but currently I cannot come up with one.

@waylan waylan added requires-changes Awaiting updates after a review. and removed needs-review Needs to be reviewed and/or approved. labels Oct 29, 2018
@waylan
Copy link
Member

waylan commented Oct 29, 2018

I agree that any read-world failure would likely be from a rather contrived edge case that we are not likely to think up ahead of time. That said, I think we need to add some test to show that we at least considered it.

@facelessuser
Copy link
Collaborator Author

That said, I think we need to add some test to show that we at least considered it.

I agree. Hopefully I'll get to it later today.

@waylan waylan added approved The pull request is ready to be merged. and removed requires-changes Awaiting updates after a review. labels Oct 30, 2018
@facelessuser
Copy link
Collaborator Author

Requested check is in and is passing as expected.

@waylan
Copy link
Member

waylan commented Oct 30, 2018

I just realized this needs a comment in the release notes. A one-liner under bug fixes should suffice.

@facelessuser
Copy link
Collaborator Author

Added.

@waylan waylan merged commit a38fd1b into Python-Markdown:master Oct 30, 2018
@waylan waylan added this to the Version 3.1 milestone Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved The pull request is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Line break in link references inside lists
3 participants