git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 --]

      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).