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

Option to select specific fixer to run #278

Closed
cgl opened this issue Oct 26, 2022 · 7 comments · Fixed by #443
Closed

Option to select specific fixer to run #278

cgl opened this issue Oct 26, 2022 · 7 comments · Fixed by #443

Comments

@cgl
Copy link

cgl commented Oct 26, 2022

Description

It would be nice to have the option to specify one or more fixer to apply.
One usecase is to be able to categorise the changes in several different commits, and another usecase is the codebase might not be ready to apply some of the fixes but others.

@adamchainz
Copy link
Owner

Yeah that would be nice. I think we could take module names to select like --fixer request_headers. Would you like to work on a PR?

@adamchainz
Copy link
Owner

And perhaps --no-fixer request_headers to disable a fixer, so that if someone finds a particular fixer problematic, they don't need to stop using the tool.

@adamchainz
Copy link
Owner

One workaround is to use grepdiff to selectively commit fixed lines: https://stackoverflow.com/a/52394658

davidszotten added a commit to davidszotten/django-upgrade that referenced this issue Mar 6, 2023
Add `--fixer foo` option (can be repeated) to only run specific fixers.
Fixes adamchainz#278
davidszotten added a commit to davidszotten/django-upgrade that referenced this issue Mar 6, 2023
Add `--fixer foo` option (can be repeated) to only run specific fixers.
Fixes adamchainz#278
davidszotten added a commit to davidszotten/django-upgrade that referenced this issue Mar 7, 2023
Add `--fixer foo` option (can be repeated) to only run specific fixers.
Fixes adamchainz#278
davidszotten added a commit to davidszotten/django-upgrade that referenced this issue Mar 7, 2023
Add `--fixer foo` option (can be repeated) to only run specific fixers.
Fixes adamchainz#278
@gav-fyi
Copy link
Contributor

gav-fyi commented Apr 9, 2024

Hi @adamchainz

I have picked up this issue, following on from a discussion on PR#333. I will Take David's PR, and expand on it for the additions that you requested:

  • Rename the option to --only
  • Add an opposing --skip option to skip named fixers. This would users work around when one fixer makes bad changes in their code base.
  • Add a --list-fixers option to list the fixers.
  • Documentation for all the above.
  • Unit tests for all of the above

I just have a couple of questions about the implementation/scope though:

  1. Do you have a preference for how you would like the --list-fixers to be displayed?
    • A flat list of names
    • A table of name, min-version, and description
    • Something else
  2. If we want to tabulate the output, I presume we’d not want to use new dependencies such as rich to enable this.
    A third-party library may be overkill for what can be solved with some basic max() and ljust() use.
  3. To enable easier use of --skip and --only, is there any desire to give each of the fixers identifiers? Similar to flake8 rules, so compatibility_imports could be DJUP001 for example?
  4. May be out of scope for this Issue/PR, but is there any desire to implement pyproject.toml support so that projects can configure their --skip’d fixers?

Thanks,
Gav

@UnknownPlatypus
Copy link
Contributor

To enable easier use of --skip and --only, is there any desire to give each of the fixers identifiers? Similar to flake8 rules, so compatibility_imports could be DJUP001 for example?

I'd be in favor of keeping compatibility_imports verbose name. Identifiers like DJUP001 completely hide what the fixer is doing. Pylint also use this style (with a dash version instead of underscore) and ruff is switching too

@gav-fyi
Copy link
Contributor

gav-fyi commented Apr 9, 2024

I'd be in favor of keeping compatibility_imports verbose name. Identifiers like DJUP001 completely hide what the fixer is doing. Pylint also use this style (with a dash version instead of underscore) and ruff is switching too

Thanks for the links @UnknownPlatypus - I should probably clarify that I would still keep the verbose names, but have the codes as an option too. That being said, I'm not overly precious about the addition if others think it best to stick with what we already have.

@adamchainz
Copy link
Owner

  • Do you have a preference for how you would like the --list-fixers to be displayed?

    • A flat list of names
    • A table of name, min-version, and description
    • Something else

Keep it simple: a flat list of names, one per line, no header.

  • To enable easier use of --skip and --only, is there any desire to give each of the fixers identifiers? Similar to flake8 rules, so compatibility_imports could be DJUP001 for example?

Just the full names of the fixer modules. No codes, for the reasons @UnknownPlatypus gave. We might rename some as part of this, to make their names clearer.

  • May be out of scope for this Issue/PR, but is there any desire to implement pyproject.toml support so that projects can configure their --skip’d fixers?

No desire right now. Most users (probably) use pre-commit, for which args can be used. In other cases, there’s normally another wrapper tool like tox that can add CLI arguments.

adamchainz added a commit that referenced this issue May 10, 2024
Fixes #278.

Co-authored-by: Adam Johnson <me@adamj.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants