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

log: add option to search for header or body #1710

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vegerot
Copy link

@vegerot vegerot commented Apr 5, 2024

cc: Phillip Wood [email protected]
cc: strager [email protected]
cc: me [email protected]

@vegerot
Copy link
Author

vegerot commented Apr 5, 2024

Note to reviewer: please help me come up with a better name than --header-or. I want this option to be easily discoverable by someone like me, who is looking for commits either authored or co-authored by someone

@vegerot
Copy link
Author

vegerot commented Apr 5, 2024

/preview

Copy link

gitgitgadget bot commented Apr 5, 2024

Preview email sent as [email protected]

@vegerot
Copy link
Author

vegerot commented Apr 5, 2024

/submit

Copy link

gitgitgadget bot commented Apr 5, 2024

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1710/vegerot/header-or-v1

To fetch this version to local tag pr-1710/vegerot/header-or-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1710/vegerot/header-or-v1

Copy link

gitgitgadget bot commented Apr 6, 2024

On the Git mailing list, Phillip Wood wrote (reply to this):

Hi Max

On 05/04/2024 22:48, Max Coplan via GitGitGadget wrote:
> From: =?UTF-8?q?Max=20=F0=9F=91=A8=F0=9F=8F=BD=E2=80=8D=F0=9F=92=BB=20Copl?=
>   =?UTF-8?q?an?= <[email protected]>
> > Note to reviewer: I hate the name `--header-or`!  Please help me come up
> with a better name.
> > Summary:
> This change adds a new option to `git log` that allows users to search
> for commits that match either the author or the commit message. This is
> useful for finding commits that were either authored or co-authored by a
> specific person.

I think that having a way to find all the commits that are authored by, or have a specific trailer naming someone is a good idea. I'm not sure about combining "--author" with "--grep" to do it though. Perhaps we could design a new option (tentatively called "--indent") that allows the user to specify a name and which trailers and commit headers (author and/or committer) to match.

A couple of house keeping notes. We do not use the "feat(...)" style of commit messages in this project. The subject line should just start with "log: " the rest of the subject looks fine. I think everything under the "Summary:" line above does a good job of explaining why the change is useful which is great. We tend to avoid emoji's in peoples names as well.

> [...]
> +test_expect_success 'log --grep --author --header-or uses union' '
> +	# grep matches only third and fourth
> +	# author matches only initial and third

I'm a bit confused by this comment as we seem to only expect "third" and "fourth" below - what happened to "initial"?

> +	git log --author="A U Thor" --grep=r --header-or --format=%s >actual &&
> +	{
> +	    echo fourth && echo third
> +	} >expect &&

We have a test helper to help with this

    test_write_lines fourth third >expect

Best Wishes

Phillip

Copy link

gitgitgadget bot commented Apr 6, 2024

User Phillip Wood <[email protected]> has been added to the cc: list.

Copy link

gitgitgadget bot commented Apr 7, 2024

There are issues in commit f38e52f:
log: add option to search for header or body
Lines in the body of the commit messages should be wrapped between 60 and 76 characters.
Indented lines, and lines without whitespace, are exempt

Summary:
This change adds a new option to `git log` that allows users to search
for commits that match either the author or the commit message. This is
useful for finding commits that were either authored or co-authored by a
specific person.

Currently, the best way to find a commit either authored or co-authored
by a specific person is to use

```sh
$ echo \
    $(git log --author=Torvalds --pretty="%cd,%H\n" --date=iso-strict) \
    $(git log --grep="Co-authored-by: .*Torvalds" --pretty="%cd,%H\n" --date=iso-strict) \
| sort -n --reverse \
| awk -F, '{print $2}' \
| tr '\n' '\t' \
| xargs git show --stat --stdin
```

This is a bit of a pain, so this change adds a new option to `git log`.
Now finding either authors or co-authors is as simple as

```sh
$ git log --author=Torvalds --grep=Torvalds --match-header-or-grep
```

Test plan:
1. create commit authored by A and co-authored-by B
2. create commit authored by B
3. run
```sh
$ git log --author=B --grep="Co-authored-by: B" --match-header-or-grep
```
4. expect to see both commits

Signed-off-by: Max 👨🏽‍💻 Coplan <[email protected]>
@vegerot vegerot changed the title feat(log): add option to search for header or body to git log log: add option to search for header or body Apr 7, 2024
@vegerot
Copy link
Author

vegerot commented Apr 7, 2024

/submit

Copy link

gitgitgadget bot commented Apr 7, 2024

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1710/vegerot/header-or-v2

To fetch this version to local tag pr-1710/vegerot/header-or-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1710/vegerot/header-or-v2

Copy link

gitgitgadget bot commented Apr 7, 2024

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Max Coplan via GitGitGadget" <[email protected]> writes:

> This change adds a new option to `git log` that allows users to search
> for commits that match either the author or the commit message. This is
> useful for finding commits that were either authored or co-authored by a
> specific person.

I have this feeling that the "solution" presented is not quite
addressing the use case in a more useful and direct way than it
could be.  When I designed how the --author/--committer restriction
and --grep in the body of the message interact, I made a concious
decision that "among those commits that were authored by person X,
find the ones that mention Y" is far more useful than "done by X, or
done by anybody that mention Y", especially when Y is just a text
search in the free form.  There was nothing that limits the mention
of Y to those specifically involved in the commit---the mention could
just have been part of text, like "earlier Max Coplan sent a patch,
but this commit is not related to it".

But these days, we have a more established "convention" that lists
people at the end in the form of "trailers", and that changes the
picture quite a lot from how the world order was back then.

In other words, if the true objective is to find commits that
involved person X, Y or Z (which is very common and would be a lot
more useful than finding those that involve all of them), shouldn't
we be limiting the --grep side even further so that a random mention
of person Y is excluded and hit is counted only when person Y is
mentioned on a trailer (while loosening the --author side so that it
is OR'ed instead of AND'ed)?

I am imagining a pair of new options to name people (all OR'ed) and
to name places the names of these people should appear (again, all
OR'ed).  I am not good at naming, so the option names in the example
is not more than illustration of an idea and not my recommendation,
but a command:

    git log --by="Max Coplan" --by="Junio C Hamano" \
	    --by-where=author,Signed-off-by,Co-authored-by

would find a commit that has one (or more) of the given names
in one (or more) of the places that are specified, where the places
can be either "author", "committer" to specify these headers in the
commit object, or random other string to specify trailer lines with
given keys.

Hmm?

Copy link

gitgitgadget bot commented Apr 7, 2024

On the Git mailing list, Phillip Wood wrote (reply to this):

On 07/04/2024 07:08, Junio C Hamano wrote:
> "Max Coplan via GitGitGadget" <[email protected]> writes:
> I am imagining a pair of new options to name people (all OR'ed) and
> to name places the names of these people should appear (again, all
> OR'ed).  I am not good at naming, so the option names in the example
> is not more than illustration of an idea and not my recommendation,
> but a command:
> >      git log --by="Max Coplan" --by="Junio C Hamano" \
> 	    --by-where=author,Signed-off-by,Co-authored-by
> > would find a commit that has one (or more) of the given names
> in one (or more) of the places that are specified, where the places
> can be either "author", "committer" to specify these headers in the
> commit object, or random other string to specify trailer lines with
> given keys.

I like this, yesterday[1] I didn't have a clear idea in my head of how such an option should work but I think passing the names and the fields to match those names against as two separate options is a good idea.

Best Wishes

Phillip

[1] https://lore.kernel.org/git/[email protected]/

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

Successfully merging this pull request may close these issues.

1 participant