Skip to content

Add feature to only changes that were already modified (git) #1324

Open
@vinipsmaker

Description

@vinipsmaker

Hi, could you add a feature to not produce noisy git diffs? Thanks.

[edited for tone, by nrc]

Activity

nrc

nrc commented on Feb 19, 2017

@nrc
Member

If somebody wanted to work on this, then I'd be happy to take a PR. But it would be an interesting (big) project. It would mean adding knowledge of Git (and presumably Hg and other VCSs) to Rustfmt and would mean making the Rustfmt work on only sections of code (which it can't do today). It would also be imperfect because at a minimum we have to operate on a whole AST node, not literally just a chunk of text.

vinipsmaker

vinipsmaker commented on Feb 19, 2017

@vinipsmaker
Author

It would also be imperfect because at a minimum we have to operate on a whole AST node, not literally just a chunk of text.

Why would it be imperfect? I don't get. You still should work on the AST. Just don't change the tokens/text that match the "sacred" lines.

nrc

nrc commented on Feb 19, 2017

@nrc
Member

Why would it be imperfect?

Because Rustfmt might change the number of lines or the content of lines outside the diff. In the former case, this makes it hard (perhaps not impossible, but hard - consider that git diff is trying to solve the same problem (matching changes to lines) and often gets it quite wrong. It is easier for Rustfmt since it is only a single change, but still very hard). The second change means it might not be possible to do this. Consider the following:

foo_bar(arg1, arg2,
        arg3, arg4);  // Only this line is in the diff.

Rustfmt has to work on the node whole node, and makes:

foo_bar(
    arg1,
    arg2,
    arg3,
    arg4,
};

If we restrict to the changed lines we get:

foo_bar(arg1, arg2,
    arg3,

Which doesn't compile. One can create examples that do compile but have different behaviour. You might suppose we could track the tokens we actually change, but this is not done and would be a huge change (and since we can move tokens as well as change the whitespace around them, is much harder than it appears). Finally, even if we managed, some how, to achieve all this, then hopefully the changing indents in the above example shows how by formatting only some lines in a node, we might actually make formatting worse instead of better.

garyyu

garyyu commented on Sep 18, 2018

@garyyu

I love rustfmt, it's great 👍 But indeed, rustfmt's code changing on untouched code of a git commit is an evil for code review (for this git commit), heavy headache on this.

Hope somebody can take this issue and let rustfmt have this feature!

scampi

scampi commented on Apr 11, 2019

@scampi
Contributor

For the git we could leverage https://crates.io/crates/git2. In addition, this feature could rely on the file_lines option to offload most of the work:

  • get the diff from git
  • turn the diff into the format understood by file_lines
zroug

zroug commented on Apr 11, 2019

@zroug

I use a script that is very similar to this approach for myself. It's far from perfect, both because the script itself isn't perfect and because file_lines doesn't always work perfectly. But sometimes I find it very useful as a temporary workaround. I haven't added error handling or anything.

https://gist.github.com/zroug/28605f45a662b483fb4a7c3545627f66

eddyb

eddyb commented on Jun 11, 2019

@eddyb
Member

@zroug That works great, I just had to add "--edition=2018" to the args list.

added a commit that references this issue on Jun 12, 2019

Auto merge of #61722 - eddyb:vowel-exclusion-zone, r=oli-obk

6 remaining items

cormacrelf

cormacrelf commented on Nov 10, 2022

@cormacrelf

If your problem is noisy diffs, AND you've already run cargo fmt on your unstaged changes, then a line-based-rustfmt solution isn't enough. That's super common with format-on-save plugins or muscle-memory keybindings, if you've forgotten, or if you've had formatting disabled for a while (I recently used a stage1 build of rustc for a while, so no rustfmt). A way to salvage this is my tool git-index-exec: it can run things like cargo fmt on a copy of your index, and write any changes back to your index, never touching your working directory. Some scenarios:

  • You've accidentally run cargo fmt and now you can't find your work in the diff noise. Run git index-exec 'cargo fmt', and git diff now shows what it did before. The lines that did change have also been formatted. You can now commit the formatting by itself, and continue working.
  • You forgot to format your last commit. Simply cargo-fmt both the working directory and the index, and commit the formatted index with --amend.
  • You want a pre-commit hook cargo fmt --check that doesn't require unstaged changes to be formatted correctly. Add git index-exec 'cargo fmt --check --quiet' || exit $? or similar to your .git/hooks/pre-commit file.
  • You want to know if what you're about to commit compiles, but you don't know if you have staged all of the relevant changes to make it compile on its own. (For example, a rename operation that touched some other file.) git index-exec 'cargo check' does the job. You can also add this as a pre-commit hook, which will similarly be much more correct than using cargo-check on the working directory.

Under the hood, it's just using a git worktree in /tmp, which stays alive between usages so that subsequent check/clippy runs are fast. There's not much to it, but once you have it, everything looks like a hammer. The biggest upside is that it means the solution to your formatting problems always involves more formatting, not trying to restrict the application. It just lets you point the unstoppable train that is rustfmt at more useful targets.

tgross35

tgross35 commented on Feb 3, 2023

@tgross35
Contributor

After having worked with a codebase that does incremental formatting... it's difficult. There are all kinds of annoying edge cases (is this the right indentation level?), style might not be coherent with surrounding content, and enforcing on CI can be tricky.

A better solution is to perform a one-time formatting of your entire codebase, then add the commit hash to a .git-blame-ignore-revs file. This is supported by both GitHub and GitLab and will hide that commit from the blame viewer, so you don't wind up with untraceable history.

After that you can just use rustfmt as-is without worrying about incremental formatting, and your blames won't have the noise of the big formatting commit.

deleted a comment from vinipsmaker on Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @eddyb@cormacrelf@nrc@scampi@vinipsmaker

        Issue actions

          Add feature to only changes that were already modified (git) · Issue #1324 · rust-lang/rustfmt