Skip to content

Re-use existing regex for whitespace detection#1368

Merged
davidism merged 1 commit intopallets:mainfrom
mvolfik:refactor-regex
Mar 8, 2022
Merged

Re-use existing regex for whitespace detection#1368
davidism merged 1 commit intopallets:mainfrom
mvolfik:refactor-regex

Conversation

@mvolfik
Copy link
Copy Markdown
Contributor

@mvolfik mvolfik commented Mar 10, 2021

Less regexes = win.
Whitespace regex could also detect newlines, but since we're only
matching the part from last newline, it's safer (and more readable)
to use it here.

Checklist:

  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@davidism
Copy link
Copy Markdown
Member

Fairly sure that comment/code you removed was due to a previous PR. Can you check to see if you can find that PR so we can make sure we're not missing something?

@mvolfik
Copy link
Copy Markdown
Contributor Author

mvolfik commented Mar 10, 2021

You just added comments, some variables and reformatted in 37249c0 There was some change in dbbd082, but this PR doesn't reintroduce the bug in the linked issue (and I suppose there is a test for that somewhere). The regex was introduced in 7d00a40, I don't know what was going on there before that. So I don't think I'm reverting some old PR.

@davidism
Copy link
Copy Markdown
Member

davidism commented Apr 6, 2021

#858, #1014, and #1183 seem relevant and important enough to warrant a second look at why the code is the way it is.

@davidism davidism added this to the 3.1.0 milestone Mar 8, 2022
Whitespace regex could also detect newlines, but since we're only
matching the part from last newline, it's safer (and more readable)
to use it here.
@davidism davidism merged commit 4bbb1fb into pallets:main Mar 8, 2022
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants