adamchainz/blacken-docs

"--dry-run" flag

jvzammit opened this issue · 7 comments

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.

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.

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?

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).

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.

@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

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.

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!