From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jared Van Bortel <cebtenzzre@gmail.com>
Subject: Re: [PATCH] diff: don't crash with empty argument to -G or -S
Date: Tue, 18 Feb 2025 19:29:03 +0000 [thread overview]
Message-ID: <Z7TffynGALJM4KfH@tapette.crustytoothpaste.net> (raw)
In-Reply-To: <xmqqbjuzxgn3.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]
On 2025-02-18 at 18:16:32, Junio C Hamano wrote:
> I agree BUG is unwelcome. I am not sure about the value of
> forbidding an empty string (I am sure about forbidding NULL,
> though).
>
> If an empty matches everything, "git log -S" would skip changes that
> would keep the number of lines, right? For the history of a project
> that keeps track of source code, such a "feature" would not be
> useful, but I can see a complaint by somebody who may want to keep
> track of a "list of things" one-item-per-line, if we had been
> allowing an empty string. It would be a regression for such a niche
> user.
I actually just ran a `git grep -e ''` to see what it does, and it
does indeed match every line, so presumably `git log -G` would do so as
well.
I do see your argument that this could be useful for a limited number of
use cases, but as someone who often keeps track of lists of things in
text files and therefore could be a target for that feature, I still
feel like this would be very much a corner case.
> Luckily, since we have stopped with a "BUG", we do not have to worry
> about backward compatibility in this case ;-)
I agree. The good news is that we haven't broken anyone's workflow,
unless their workflow involves trying to trigger bugs.
> So I'd say that it may be a bit premature for us to declare
> "anything useful", I am perfectly fine with the patch given here.
> If somebody who wants to maintain a text file, one-item-per-line
> that keeps track of a list of things to omit commits that do not
> change the number of items, they can drop "&& !*arg" part, tweak the
> message and add their own tests, once this fix lands and the dust
> settles.
Exactly. If there's one thing I've learned, it's that there are lots of
users who will try new things, and I'm sure we'll get a report here or
elsewhere that they'd like to add this feature if there's actually
interest. Fortunately, I expect that it shouldn't be too hard to add
such a feature.
--
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
prev parent reply other threads:[~2025-02-18 19:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-17 1:24 Crash on empty pickaxe argument Jared Van Bortel
2025-02-17 1:58 ` brian m. carlson
2025-02-17 17:57 ` [PATCH] diff: don't crash with empty argument to -G or -S brian m. carlson
2025-02-17 22:18 ` Elijah Newren
2025-02-18 18:16 ` Junio C Hamano
2025-02-18 19:29 ` brian m. carlson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z7TffynGALJM4KfH@tapette.crustytoothpaste.net \
--to=sandals@crustytoothpaste.net \
--cc=cebtenzzre@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).