-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add support for the --output-dir and --add-suffix options from 2to3 #160
base: master
Are you sure you want to change the base?
Conversation
These options were added to 2to3 in Python 2.7 and 3.2. This commit adds the options always, with descriptive errors if the user attempts to use them with a version of Python that is too old. Most of the code here was shamelessly stolen from lib2to3's source.
Yep, I think it's OK to simplify it and assume it's run by a moderately recent Python version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this has got a merge conflict now as well - could you rebase it or merge from master to fix the conflict?
@@ -27,6 +27,8 @@ def format_usage(usage): | |||
"""Method that doesn't output "Usage:" prefix""" | |||
return usage | |||
|
|||
_lib2to3_has_output_dir = hasattr(StdoutRefactoringTool([], {}, False, False, False), '_output_dir') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can drop this check - we don't aim to support the older Pythons that don't have this.
# but not replaced. | ||
if options.output_dir and not options.nobackups: | ||
parser.error("Can't use --output-dir/-o without -n.") | ||
if options.add_suffix and not options.nobackups: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These seem like strange checks. Does 2to3 do the same thing? Can we just make either of these options imply nobackups
? The backups are only needed because it overwrites the source files - if you tell it not to do that, there's no need to think about backups.
rt = StdoutRefactoringTool(sorted(fixer_names), flags, sorted(explicit), | ||
options.nobackups, not options.no_diffs) | ||
options.nobackups, not options.no_diffs, **newer_options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified when we assume that we're on a new enough Python.
@shaib there were some changes with code style, and we now use fissix and py>=3.6 and so we can be sure these flags always exist |
Should this PR be updated for fissix or closed? |
FWIW I no longer have interest in this PR. |
These options were added to 2to3 in Python 2.7 and 3.2. This commit
adds the options always, with descriptive errors if the user attempts
to use them with a version of Python that is too old.
Most of the code here was shamelessly stolen from lib2to3's source.
I note that Python 2.6 and 3.0<=Python<=3.3 are no longer supported by the project (according to the travis config), so the code may be simplified by just assuming the newer versions.
Another alternative I considered was to only add the options where they can be handled. I decided against it in the interest of discoverability, but if the decision is to keep some support for older Pythons, that is also a valid way to go.