Re: [PATCH v2 1/2] blame: respect .git-blame-ignore-revs automatically
From: Kristoffer Haugsbakk <hidden>
Date: 2024-10-12 13:58:24
Hi Abhijeetsingh For what it’s worth here’s how I imagine this feature could work conceptually: Before this feature/change, the effective config for Git use looks like this:
[blame]
No `blame.ignoreRevsFile`. But with/after it:
[blame]
ignoreRevsFile=.git-blame-ignore-revs
This is the effective config. Not what the user has typed out. If the user types out this:
[blame]
ignoreRevsFile=.git-blame-more-revs
Then this becomes their effective config:
[blame]
ignoreRevsFile=.git-blame-ignore-revs
ignoreRevsFile=.git-blame-more-revs
Now there are two files: the default one and the user-supplied one (this config variable is documented as being multi-valued: “This option may be repeated multiple times.”). § How to ignore this new default §§§ Considering users who do not want this new default:
[blame]
ignoreRevsFile=
This is the change they would have to make. Because a blank/empty resets/empties the list of files. On Sat, Oct 12, 2024, at 06:37, Abhijeetsingh Meena via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: Abhijeetsingh Meena <redacted> git-blame(1) can ignore a list of commits with `--ignore-revs-file`. This is useful for marking uninteresting commits like formatting changes, refactors and whatever else should not be “blamed”. Some projects even version control this file so that all contributors can use it; the conventional name is `.git-blame-ignore-revs`. But each user still has to opt-in to the standard ignore list, either with this option or with the config `blame.ignoreRevsFile`. Let’s teach git-blame(1) to respect this conventional file in order to streamline the process. Signed-off-by: Abhijeetsingh Meena <redacted> --- builtin/blame.c | 8 ++++++++ t/t8015-blame-default-ignore-revs.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100755 t/t8015-blame-default-ignore-revs.shdiff --git a/builtin/blame.c b/builtin/blame.c index e407a22da3b..1eddabaf60f 100644 --- a/builtin/blame.c +++ b/builtin/blame.c@@ -1105,6 +1105,14 @@ parse_done: add_pending_object(&revs, &head_commit->object, "HEAD"); } + /* + * By default, add .git-blame-ignore-revs to the list of files + * containing revisions to ignore if it exists. + */ + if (access(".git-blame-ignore-revs", F_OK) == 0) { + string_list_append(&ignore_revs_file_list, ".git-blame-ignore-revs"); + } +
I have not tested these patches. But I see why you check for file access/existence. Because with this config:
[blame]
ignoreRevsFile=.git-blame-ignore-revs
I get this warning in repositories that don’t have the file:
fatal: could not open object name list: .git-blame-ignore-revs
Which is just noise. I get the same thing with Git Notes namespace configurations. I need to configure them for certain repositories (like `amlog` in this project), but then I get warnings about them when using the relevant commands in a project that does not have them. Maybe this is totally off-topic but I think it would make more sense if `blame.ignoreRevsFile` just didn’t say anything if it didn’t find the file. Because the point of the config might be to opt-in to this file for those projects that does have it.
quoted hunk ↗ jump to hunk
init_scoreboard(&sb); sb.revs = &revs; sb.contents_from = contents_from;diff --git a/t/t8015-blame-default-ignore-revs.shb/t/t8015-blame-default-ignore-revs.sh new file mode 100755 index 00000000000..d4ab686f14d--- /dev/null +++ b/t/t8015-blame-default-ignore-revs.sh@@ -0,0 +1,26 @@ +#!/bin/sh + +test_description='default revisions to ignore when blaming' + +TEST_PASSES_SANITIZE_LEAK=true +. ./test-lib.sh + +test_expect_success 'blame: default-ignore-revs-file' ' + test_commit first-commit hello.txt hello && + + echo world >>hello.txt && + test_commit second-commit hello.txt && + + sed "1s/hello/hi/" <hello.txt > hello.txt.tmp && + mv hello.txt.tmp hello.txt && + test_commit third-commit hello.txt && + + git rev-parse HEAD >ignored-file && + git blame --ignore-revs-file=ignored-file hello.txt >expect && + git rev-parse HEAD >.git-blame-ignore-revs && + git blame hello.txt >actual && + + test_cmp expect actual +' + +test_done --gitgitgadget
-- Kristoffer