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

"--dry-run" flag #197

Closed
jvzammit opened this issue Dec 2, 2022 · 7 comments
Closed

"--dry-run" flag #197

jvzammit opened this issue Dec 2, 2022 · 7 comments

Comments

@jvzammit
Copy link

jvzammit commented Dec 2, 2022

Let's say I need to "blacken docs" for a large project, case in point django/django#16261

It would be useful to separate commits into two:

  1. The manual changes, to current docs, required for blacken-docs command to work without errors.
  2. The automated changes by running blacken-docs.

I.e. Part of what @carltongibson suggested here django/django#16261 (comment)

So this issue is about allowing the user to add a -D or --dry-run flag to the current command. This would prevent the command from changing files.

@adamchainz
Copy link
Owner

adamchainz commented Jan 15, 2023

Is this really needed? You can test by undoing file changes with git restore afterwards. I am currently testing #196 with:

pre-commit run blacken-docs --all ; git restore .

I'm concerned because “dry run” can mean a lot of things. Future users might expect it to output the changed file, or the diff. This feels a little like feature bloat when all the “undo changes”/diff/etc. functionality can be achieve with Git.

@jvzammit
Copy link
Author

If I run git restore . I would lose all the changes. So for django/django#16261 I plan to do this "cycle" progressively:

  • Run blacken-docs command for all docs files to get what needs to change for blacken-docs to run
  • Apply manual changes for a specific file
  • Re-run

Repeat until blacken-docs can run without errors. Once that is reached, commit the changes done manually (first commit).

After the manual changes are committed, run blacken-docs and commit the changes done by the command (second commit).

Full context: django/django#16261 (comment)

That would make review easier for that PR. And reviewers would be able to tell whether it's a manual change or a change by blacken-docs.

I understand your concerns about the name "--dry-run". But I couldn't come up with a better name. I don't think "--fake" is a good name, for example. Any ideas?

@adamchainz
Copy link
Owner

I think you can do this:

  1. Run blacken-docs on all files to see its output from what would change.
  2. git restore .
  3. Manually fix a file and git add it.
  4. Loop.

git restore does not undo staged changes, so you can incrementally build up your manual fix commit.

I understand your concerns about the name "--dry-run". But I couldn't come up with a better name. I don't think "--fake" is a good name, for example. Any ideas?

My concern is not about the name, it’s about how these features tend towards scope creep. Whatever we would call the option future requests to pile on more “dry run” functionality might seem reasonable (show the diff, count of files changed, ...).

I find it a bit wasteful for every code formatting tool to add dry run mode, and other common features like directory recursion support, ignore file support, etc., when there are ways to achieve the same by combining tools (the Unix philosophy).

@jvzammit
Copy link
Author

Thanks @adamchainz ! I'll try using the flow you suggested (in the coming days).

I agree about scope creep and that "less is more".

So I'll close this issue. And ping you again in case the flow you suggested does not work.

@jvzammit
Copy link
Author

jvzammit commented Jan 19, 2023

@adamchainz I tried the flow.

In step 3:

Manually fix a file and git add it

The only to verify a file is fixed is by running blacken-docs, which does change the file.

This mixes my manual changes to "fix" the file with the changes by the command itself.

I need to have the command verify the file is fixed without the command further changing the file.

Is there a way to do this?

That's why I had opened #198

@adamchainz
Copy link
Owner

I think this workflow would work? You can always add more commits with Git...

  1. Write an attempted fix for a file and git add it.
  2. Run blacken-docs to see if it worked.
  3. git restore .
  4. If the file is fixed, git commit (maybe with --amend to add to a previous commit of multiple fixes).
  5. Loop.

@jvzammit
Copy link
Author

jvzammit commented Feb 3, 2023

Yeah it works @adamchainz thanks. It's just that it's more work. But this should be a "one time" effort with the docs that "do not pass" yet. So it's not worth bloating blacken-docs with another argument. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants