search

ikamensh / flynt

A tool to automatically convert old string literal formatting to f-strings
MIT License
687 stars 33 forks source link

Allow skipping lines that should not be converted #30

Closed danielhoherd closed 5 years ago

danielhoherd commented 5 years ago

In the README you point out scenarios where conversion would cause problems. It would be great if there were a configurable way to skip lines that should not be converted. For instance it should skip lines that contain # noqa. This would go great with pre-commit and CI jobs.

ikamensh commented 5 years ago

Hey @danielhoherd, this suggestion makes sense since flynt now supports pre-commit (initially I was thinking about it as a tool to do one-time conversion; in this case you would just revert some lines in git instead of creating any comments to prevent them being changed).

As I am not a regular user of flake8 / pylint / other linters, I am not very sure what are the conventions and expectations there (how do we mark a whole file to be skipped? should I skip conversions if only a particular warning is disabled, e.g. # noqa: W731). I will for now add a flag --skip-noqa which will skip any lines / multilines containing substring '# noqa'.

The reason to make it optional is that I don't want to skip lines that had QA disabled for some reason that has nothing to do with flynt.

ikamensh commented 5 years ago

Further thinking about it, perhaps the best would be to look for regex #[ ]noqa.flynt. I guess having noqa before introducing flynt would tell nothing about whether people want the f-string conversion on the lines or not. Now they can explicitly disable f-string conversion. Also I don't have to introduce a flag (because its very improbable people would have had these lines by chance), and this can be made the default behavior.

ikamensh commented 5 years ago

Its done: with flynt 0.33 (on pypi) you can add # noqa: [anything else] flynt to skip lines from conversion. Note that plain # noqa doesn't prevent conversion, also currently there is no way to skip the whole file. If you feel those are significant shortcomings, feel free to reopen / create new issue.