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

Update install.py #5173

Closed
wants to merge 1 commit into from
Closed

Update install.py #5173

wants to merge 1 commit into from

Conversation

Avinash-Raj
Copy link

-> Why I have changed s = re.sub(r'#.*?\n', '', s) to s = re.sub(r'#.*\n?', '', s)?

Because your regex won't match the comment line which exists at the last (End of a file) because it expects a newline character . But s = re.sub(r'#.*\n?', '', s) should remove all the comment lines irrespective of their locations. .*? -> .* because dot by default won't match line breaks.

-> A simple string.replace should do this re.sub(r'\'', '"', s) job, you don't need to go for `re.sub

If you want to further reduce these two lines of code into a single line then go with this,

re.sub(r"#.*\n?|(')", lambda x: '"' if x.group(1) else '', s)

@rvagg
Copy link
Member

rvagg commented Feb 10, 2016

Is there a particular case you have where there is a comment at the bottom of the file? The change lgtm and seems more correct (.*? is pretty amusing) but I'm wondering if it's addressing an actual problem.

@mscdex mscdex added the tools Issues and PRs related to the tools directory. label Feb 10, 2016
@Fishrock123
Copy link
Contributor

Closing, @Avinash-Raj if you can show somewhere that could be affected by this we'll re-open. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants