| Message ID | pull.1809.git.1728370892696.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | blame: respect .git-blame-ignore-revs automatically | expand |
Thank you for working on this. I think an ability to respect a
conventional ignore list would be useful.
I was a bit concerned that there doesn’t seem to be something to turn
off ignore lists. The man page says that the discussed option exists
but not some `--no` variant. But then I tested this:
```
git blame --ignore-revs-file=README.md \
--no-ignore-revs-file README.md
```
And that works without giving errors. So it’s there. Just apparently
undocumented?
My assumption then is that, with this change, I could use
`--no-ignore-revs-file` to turn off the default file.
(One can also use the config `blame.ignoreRevsFile` set to the
empty string)
On Tue, Oct 8, 2024, at 09:01, Abhijeetsingh Meena via GitGitGadget wrote:
> From: Abhijeetsingh Meena <abhijeet040403@gmail.com>
>
> Modify `git blame` to automatically respect a `.git-blame-ignore-revs`
> file if it exists in the repository. This file is used by many projects
> to ignore non-functional changes, such as reformatting or large-scale
> refactoring, when generating blame information.
Nice.
> Before this change, users had to manually specify the file with the
> `--ignore-revs-file` option. This update streamlines the process by
> automatically detecting the `.git-blame-ignore-revs` file, reducing
> manual effort.
(They can also use the `blame.ignoreRevsFile` config in order to not
have to give the option every time)
The project convention is to use the present tense for describing the
current behavior (without this patch). Like:
Users that have a standard list of commits to ignore have to use
`--ignore-revs-file`.
This style often lends itself to this structure:
- (optionally introduce the wider topic)
- Describe the current behavior and why that is a problem
- Say how we’re changing the code (which you already did a good job of in
your first paragraph).
It might end up something like this:
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.
> This change aligns with the standardized practice in many repositories
> and simplifies the workflow for users.
>
> Signed-off-by: Abhijeetsingh Meena <abhijeet040403@gmail.com>
> ---
> blame: respect .git-blame-ignore-revs automatically
>
>
> Introduction
> ============
>
> Hi, I'm Abhijeet (Ethan0456), and this is my first contribution to the
> Git project. I currently work as an ML Engineer at an early-stage
> startup, and I’m excited to contribute to this open-source project.
>
>
> Why the Change?
> ===============
>
> I came across this enhancement request on the bug tracker and found it
> beginner-friendly, making it a great opportunity for me to get familiar
> with the Git codebase. The ability for git blame to automatically
> respect the .git-blame-ignore-revs file is something that can streamline
> workflows for many users, and I felt it would be a valuable addition.
The issue that is referred:
https://github.com/gitgitgadget/git/issues/1494
That issue tracker is unofficial by the way.
>
>
> Feedback
> ========
>
> While I’m confident in the changes made to builtin/blame.c and the new
> test case in t/t8015-blame-ignore-revs.sh, I welcome any feedback or
> suggestions to improve both my code and approach. I’m eager to learn
> from the community and improve where needed.
>
>
> Community Need
> ==============
>
> There is precedent for this functionality in other projects:
>
> * Chromium
>
> [https://chromium.googlesource.com/chromium/src.git/+/f0596779e57f46fccb115a0fd65f0305894e3031/.git-blame-ignore-revs],
> which powers many popular browsers, uses .git-blame-ignore-revs
> to
> simplify the blame process by ignoring non-functional changes.
> * Rob Allen's blog post
> [https://akrabat.com/ignoring-revisions-with-git-blame/]
> discusses
> the need for ignoring revisions with git blame, and a commenter
> specifically suggests that it would be helpful if Git
> automatically
> respected .git-blame-ignore-revs.
>
> I hope this change aligns with community needs and improves the git
> blame experience for users.
It’s nice that you mention specific projects that use this convention.
Sometimes people just say “this is used by many people/projects”. But
not what projects. (Then perhaps it is eventually revealed that “we use
it so therefore it is important” ;) )
> Published-As:
> https://github.com/gitgitgadget/git/releases/tag/pr-1809%2FEthan0456%2Fblame-auto-ignore-revs-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
> pr-1809/Ethan0456/blame-auto-ignore-revs-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1809
>
> 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.sh
>
> diff --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");
> + }
> +
> init_scoreboard(&sb);
> sb.revs = &revs;
> sb.contents_from = contents_from;
> diff --git a/t/t8015-blame-default-ignore-revs.sh
> b/t/t8015-blame-default-ignore-revs.sh
> new file mode 100755
> index 00000000000..84e1a9e87e6
> --- /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
> \ No newline at end of file
There should be a final newline. ;)
>
> base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
> --
> gitgitgadget
Hi Abhijeetsingh On 08/10/2024 08:01, Abhijeetsingh Meena via GitGitGadget wrote: > From: Abhijeetsingh Meena <abhijeet040403@gmail.com> > > Modify `git blame` to automatically respect a `.git-blame-ignore-revs` > file if it exists in the repository. This file is used by many projects > to ignore non-functional changes, such as reformatting or large-scale > refactoring, when generating blame information. > > Before this change, users had to manually specify the file with the > `--ignore-revs-file` option. This update streamlines the process by > automatically detecting the `.git-blame-ignore-revs` file, reducing > manual effort. As Kristoffer mentioned there is a config option so that the user does not have to specify the file each time. The decision not to support a default file for this feature was deliberate [1,2]. It is possible that we now have enough experience with the feature that we think it would be a good idea but any such change would need to address the concerns about ignoring "cleanup" commits that introduce bugs. If we do decide to support a default file we'd need to work out how it interacted with the config setting and make sure there was an easy way to turn the feature off. Best Wishes Phillip [1] https://lore.kernel.org/git/c34159af-f97e-82b3-e2a1-04adae5c10ac@google.com/ [2] https://lore.kernel.org/git/YLfe+HXl4hkzs44b@nand.local/ > This change aligns with the standardized practice in many repositories > and simplifies the workflow for users. > > Signed-off-by: Abhijeetsingh Meena <abhijeet040403@gmail.com> > --- > blame: respect .git-blame-ignore-revs automatically > > > Introduction > ============ > > Hi, I'm Abhijeet (Ethan0456), and this is my first contribution to the > Git project. I currently work as an ML Engineer at an early-stage > startup, and I’m excited to contribute to this open-source project. > > > Why the Change? > =============== > > I came across this enhancement request on the bug tracker and found it > beginner-friendly, making it a great opportunity for me to get familiar > with the Git codebase. The ability for git blame to automatically > respect the .git-blame-ignore-revs file is something that can streamline > workflows for many users, and I felt it would be a valuable addition. > > > Feedback > ======== > > While I’m confident in the changes made to builtin/blame.c and the new > test case in t/t8015-blame-ignore-revs.sh, I welcome any feedback or > suggestions to improve both my code and approach. I’m eager to learn > from the community and improve where needed. > > > Community Need > ============== > > There is precedent for this functionality in other projects: > > * Chromium > [https://chromium.googlesource.com/chromium/src.git/+/f0596779e57f46fccb115a0fd65f0305894e3031/.git-blame-ignore-revs], > which powers many popular browsers, uses .git-blame-ignore-revs to > simplify the blame process by ignoring non-functional changes. > * Rob Allen's blog post > [https://akrabat.com/ignoring-revisions-with-git-blame/] discusses > the need for ignoring revisions with git blame, and a commenter > specifically suggests that it would be helpful if Git automatically > respected .git-blame-ignore-revs. > > I hope this change aligns with community needs and improves the git > blame experience for users. > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1809%2FEthan0456%2Fblame-auto-ignore-revs-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1809/Ethan0456/blame-auto-ignore-revs-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1809 > > 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.sh > > diff --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"); > + } > + > init_scoreboard(&sb); > sb.revs = &revs; > sb.contents_from = contents_from; > diff --git a/t/t8015-blame-default-ignore-revs.sh b/t/t8015-blame-default-ignore-revs.sh > new file mode 100755 > index 00000000000..84e1a9e87e6 > --- /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 > \ No newline at end of file > > base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2
Hi Kristoffer, Thank you so much for your feedback and for taking the time to review my patch. I appreciate your suggestions, particularly regarding the project convention of using the present tense to describe the current behavior, as well as your mention of the --no-ignore-revs-file option and the importance of documenting it. Regarding your first point: > My assumption then is that, with this change, I could use --no-ignore-revs-file to turn off the default file. I’ll check how this works in the current setup and get back to you to confirm. As for your second observation: > git blame --ignore-revs-file=README.md > --no-ignore-revs-file README.md > And that works without giving errors. So it’s there. Just apparently undocumented? I’ll look into opening an issue and submitting a PR to add documentation for this, as it seems important to address. I’m excited to refine the patch with these improvements in mind and will continue working on the commit message and necessary documentation in line with your feedback. Thanks again for your guidance and support! Best regards, Abhijeet (Ethan0456) On Tue, Oct 8, 2024 at 8:53 PM Abhijeetsingh Meena <abhijeetsingh.github@gmail.com> wrote: > > Hi Phillip, > > Thank you for your feedback on my patch. I appreciate your insights regarding the decision not to support a default file for the --ignore-revs-file feature and the concerns related to ignoring "cleanup" commits that could potentially introduce bugs. > > I understand that addressing these concerns is crucial. Specifically, I will focus on: > > 1. Ensuring there’s a clear and easy way for users to opt out or override the default behavior of ignoring certain commits, especially if they suspect those commits might contain bugs. > 2. Investigating how this new feature could interact with existing configuration options that allow users to specify what should be ignored manually. > > I will take these points into consideration as I work on my changes and come up with a solution to address these concerns. > > Thank you again for your guidance, and I look forward to your continued feedback! > > Best wishes, > Abhijeet (Ethan0456) > > On Tue, Oct 8, 2024 at 8:53 PM Abhijeetsingh Meena <abhijeetsingh.github@gmail.com> wrote: >> >> Hi Kristoffer, >> >> Thank you so much for your feedback and for taking the time to review my patch. I appreciate your suggestions, particularly regarding the project convention of using the present tense to describe the current behavior, as well as your mention of the --no-ignore-revs-file option and the importance of documenting it. >> >> Regarding your first point: >> >> > My assumption then is that, with this change, I could use --no-ignore-revs-file to turn off the default file. >> >> I’ll check how this works in the current setup and get back to you to confirm. >> >> As for your second observation: >> >> > git blame --ignore-revs-file=README.md >> > --no-ignore-revs-file README.md >> > And that works without giving errors. So it’s there. Just apparently undocumented? >> >> I’ll look into opening an issue and submitting a PR to add documentation for this, as it seems important to address. >> >> I’m excited to refine the patch with these improvements in mind and will continue working on the commit message and necessary documentation in line with your feedback. >> >> Thanks again for your guidance and support! >> >> Best regards, >> Abhijeet (Ethan0456) >> >> On Tue, Oct 8, 2024 at 7:54 PM Abhijeetsingh Meena via GitGitGadget <gitgitgadget@gmail.com> wrote: >>> >>> From: Abhijeetsingh Meena <abhijeet040403@gmail.com> >>> >>> Modify `git blame` to automatically respect a `.git-blame-ignore-revs` >>> file if it exists in the repository. This file is used by many projects >>> to ignore non-functional changes, such as reformatting or large-scale >>> refactoring, when generating blame information. >>> >>> Before this change, users had to manually specify the file with the >>> `--ignore-revs-file` option. This update streamlines the process by >>> automatically detecting the `.git-blame-ignore-revs` file, reducing >>> manual effort. >>> >>> This change aligns with the standardized practice in many repositories >>> and simplifies the workflow for users. >>> >>> Signed-off-by: Abhijeetsingh Meena <abhijeet040403@gmail.com> >>> --- >>> blame: respect .git-blame-ignore-revs automatically >>> >>> >>> Introduction >>> ============ >>> >>> Hi, I'm Abhijeet (Ethan0456), and this is my first contribution to the >>> Git project. I currently work as an ML Engineer at an early-stage >>> startup, and I’m excited to contribute to this open-source project. >>> >>> >>> Why the Change? >>> =============== >>> >>> I came across this enhancement request on the bug tracker and found it >>> beginner-friendly, making it a great opportunity for me to get familiar >>> with the Git codebase. The ability for git blame to automatically >>> respect the .git-blame-ignore-revs file is something that can streamline >>> workflows for many users, and I felt it would be a valuable addition. >>> >>> >>> Feedback >>> ======== >>> >>> While I’m confident in the changes made to builtin/blame.c and the new >>> test case in t/t8015-blame-ignore-revs.sh, I welcome any feedback or >>> suggestions to improve both my code and approach. I’m eager to learn >>> from the community and improve where needed. >>> >>> >>> Community Need >>> ============== >>> >>> There is precedent for this functionality in other projects: >>> >>> * Chromium >>> [https://chromium.googlesource.com/chromium/src.git/+/f0596779e57f46fccb115a0fd65f0305894e3031/.git-blame-ignore-revs], >>> which powers many popular browsers, uses .git-blame-ignore-revs to >>> simplify the blame process by ignoring non-functional changes. >>> * Rob Allen's blog post >>> [https://akrabat.com/ignoring-revisions-with-git-blame/] discusses >>> the need for ignoring revisions with git blame, and a commenter >>> specifically suggests that it would be helpful if Git automatically >>> respected .git-blame-ignore-revs. >>> >>> I hope this change aligns with community needs and improves the git >>> blame experience for users. >>> >>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1809%2FEthan0456%2Fblame-auto-ignore-revs-v2 >>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1809/Ethan0456/blame-auto-ignore-revs-v2 >>> Pull-Request: https://github.com/gitgitgadget/git/pull/1809 >>> >>> Range-diff vs v1: >>> >>> 1: 666404681d9 = 1: 666404681d9 blame: respect .git-blame-ignore-revs automatically >>> >>> >>> 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.sh >>> >>> diff --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"); >>> + } >>> + >>> init_scoreboard(&sb); >>> sb.revs = &revs; >>> sb.contents_from = contents_from; >>> diff --git a/t/t8015-blame-default-ignore-revs.sh b/t/t8015-blame-default-ignore-revs.sh >>> new file mode 100755 >>> index 00000000000..84e1a9e87e6 >>> --- /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 >>> \ No newline at end of file >>> >>> base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2 >>> -- >>> gitgitgadget
Hi Phillip, Thank you for your feedback on my patch. I appreciate your insights regarding the decision not to support a default file for the --ignore-revs-file feature and the concerns related to ignoring "cleanup" commits that could potentially introduce bugs. I understand that addressing these concerns is crucial. Specifically, I will focus on: 1. Ensuring there’s a clear and easy way for users to opt out or override the default behavior of ignoring certain commits, especially if they suspect those commits might contain bugs. 2. Investigating how this new feature could interact with existing configuration options that allow users to specify what should be ignored manually. I will take these points into consideration as I work on my changes and come up with a solution to address these concerns. Thank you again for your guidance, and I look forward to your continued feedback! Best wishes, Abhijeet (Ethan0456) On Tue, Oct 8, 2024 at 9:42 PM Abhijeetsingh Meena <abhijeetsingh.github@gmail.com> wrote: > > Hi Kristoffer, > > Thank you so much for your feedback and for taking the time to review > my patch. I appreciate your suggestions, particularly regarding the > project convention of using the present tense to describe the current > behavior, as well as your mention of the --no-ignore-revs-file option > and the importance of documenting it. > > Regarding your first point: > > > My assumption then is that, with this change, I could use --no-ignore-revs-file to turn off the default file. > > I’ll check how this works in the current setup and get back to you to confirm. > > As for your second observation: > > > git blame --ignore-revs-file=README.md > > --no-ignore-revs-file README.md > > And that works without giving errors. So it’s there. Just apparently undocumented? > > I’ll look into opening an issue and submitting a PR to add > documentation for this, as it seems important to address. > > I’m excited to refine the patch with these improvements in mind and > will continue working on the commit message and necessary > documentation in line with your feedback. > > Thanks again for your guidance and support! > > Best regards, > Abhijeet (Ethan0456) > > On Tue, Oct 8, 2024 at 8:53 PM Abhijeetsingh Meena > <abhijeetsingh.github@gmail.com> wrote: > > > > Hi Phillip, > > > > Thank you for your feedback on my patch. I appreciate your insights regarding the decision not to support a default file for the --ignore-revs-file feature and the concerns related to ignoring "cleanup" commits that could potentially introduce bugs. > > > > I understand that addressing these concerns is crucial. Specifically, I will focus on: > > > > 1. Ensuring there’s a clear and easy way for users to opt out or override the default behavior of ignoring certain commits, especially if they suspect those commits might contain bugs. > > 2. Investigating how this new feature could interact with existing configuration options that allow users to specify what should be ignored manually. > > > > I will take these points into consideration as I work on my changes and come up with a solution to address these concerns. > > > > Thank you again for your guidance, and I look forward to your continued feedback! > > > > Best wishes, > > Abhijeet (Ethan0456) > > > > On Tue, Oct 8, 2024 at 8:53 PM Abhijeetsingh Meena <abhijeetsingh.github@gmail.com> wrote: > >> > >> Hi Kristoffer, > >> > >> Thank you so much for your feedback and for taking the time to review my patch. I appreciate your suggestions, particularly regarding the project convention of using the present tense to describe the current behavior, as well as your mention of the --no-ignore-revs-file option and the importance of documenting it. > >> > >> Regarding your first point: > >> > >> > My assumption then is that, with this change, I could use --no-ignore-revs-file to turn off the default file. > >> > >> I’ll check how this works in the current setup and get back to you to confirm. > >> > >> As for your second observation: > >> > >> > git blame --ignore-revs-file=README.md > >> > --no-ignore-revs-file README.md > >> > And that works without giving errors. So it’s there. Just apparently undocumented? > >> > >> I’ll look into opening an issue and submitting a PR to add documentation for this, as it seems important to address. > >> > >> I’m excited to refine the patch with these improvements in mind and will continue working on the commit message and necessary documentation in line with your feedback. > >> > >> Thanks again for your guidance and support! > >> > >> Best regards, > >> Abhijeet (Ethan0456) > >> > >> On Tue, Oct 8, 2024 at 7:54 PM Abhijeetsingh Meena via GitGitGadget <gitgitgadget@gmail.com> wrote: > >>> > >>> From: Abhijeetsingh Meena <abhijeet040403@gmail.com> > >>> > >>> Modify `git blame` to automatically respect a `.git-blame-ignore-revs` > >>> file if it exists in the repository. This file is used by many projects > >>> to ignore non-functional changes, such as reformatting or large-scale > >>> refactoring, when generating blame information. > >>> > >>> Before this change, users had to manually specify the file with the > >>> `--ignore-revs-file` option. This update streamlines the process by > >>> automatically detecting the `.git-blame-ignore-revs` file, reducing > >>> manual effort. > >>> > >>> This change aligns with the standardized practice in many repositories > >>> and simplifies the workflow for users. > >>> > >>> Signed-off-by: Abhijeetsingh Meena <abhijeet040403@gmail.com> > >>> --- > >>> blame: respect .git-blame-ignore-revs automatically > >>> > >>> > >>> Introduction > >>> ============ > >>> > >>> Hi, I'm Abhijeet (Ethan0456), and this is my first contribution to the > >>> Git project. I currently work as an ML Engineer at an early-stage > >>> startup, and I’m excited to contribute to this open-source project. > >>> > >>> > >>> Why the Change? > >>> =============== > >>> > >>> I came across this enhancement request on the bug tracker and found it > >>> beginner-friendly, making it a great opportunity for me to get familiar > >>> with the Git codebase. The ability for git blame to automatically > >>> respect the .git-blame-ignore-revs file is something that can streamline > >>> workflows for many users, and I felt it would be a valuable addition. > >>> > >>> > >>> Feedback > >>> ======== > >>> > >>> While I’m confident in the changes made to builtin/blame.c and the new > >>> test case in t/t8015-blame-ignore-revs.sh, I welcome any feedback or > >>> suggestions to improve both my code and approach. I’m eager to learn > >>> from the community and improve where needed. > >>> > >>> > >>> Community Need > >>> ============== > >>> > >>> There is precedent for this functionality in other projects: > >>> > >>> * Chromium > >>> [https://chromium.googlesource.com/chromium/src.git/+/f0596779e57f46fccb115a0fd65f0305894e3031/.git-blame-ignore-revs], > >>> which powers many popular browsers, uses .git-blame-ignore-revs to > >>> simplify the blame process by ignoring non-functional changes. > >>> * Rob Allen's blog post > >>> [https://akrabat.com/ignoring-revisions-with-git-blame/] discusses > >>> the need for ignoring revisions with git blame, and a commenter > >>> specifically suggests that it would be helpful if Git automatically > >>> respected .git-blame-ignore-revs. > >>> > >>> I hope this change aligns with community needs and improves the git > >>> blame experience for users. > >>> > >>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1809%2FEthan0456%2Fblame-auto-ignore-revs-v2 > >>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1809/Ethan0456/blame-auto-ignore-revs-v2 > >>> Pull-Request: https://github.com/gitgitgadget/git/pull/1809 > >>> > >>> Range-diff vs v1: > >>> > >>> 1: 666404681d9 = 1: 666404681d9 blame: respect .git-blame-ignore-revs automatically > >>> > >>> > >>> 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.sh > >>> > >>> diff --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"); > >>> + } > >>> + > >>> init_scoreboard(&sb); > >>> sb.revs = &revs; > >>> sb.contents_from = contents_from; > >>> diff --git a/t/t8015-blame-default-ignore-revs.sh b/t/t8015-blame-default-ignore-revs.sh > >>> new file mode 100755 > >>> index 00000000000..84e1a9e87e6 > >>> --- /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 > >>> \ No newline at end of file > >>> > >>> base-commit: 777489f9e09c8d0dd6b12f9d90de6376330577a2 > >>> -- > >>> gitgitgadget
diff --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"); + } + init_scoreboard(&sb); sb.revs = &revs; sb.contents_from = contents_from; diff --git a/t/t8015-blame-default-ignore-revs.sh b/t/t8015-blame-default-ignore-revs.sh new file mode 100755 index 00000000000..84e1a9e87e6 --- /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 \ No newline at end of file