Re: [PATCH v2 1/3] last-modified: rewrite error message when more than one revision given

From: Toon Claes <toon@iotcl.com>
Date: Jan 15, 2026, 11:33 UTC
Message-ID: <87jyxjjejz.fsf@iotcl.com>
In-Reply-To: <aWd2SLrBQRBC1e2S@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 16 quoted lines

> On Wed, Jan 14, 2026 at 11:24:45AM +0100, Toon Claes wrote:
>> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
>> index c80f0535f6..06e3f79aec 100644
>> --- a/builtin/last-modified.c
>> +++ b/builtin/last-modified.c
>> @@ -145,16 +145,20 @@ static int populate_paths_from_revs(struct last_modified *lm)
>>  		if (obj->item->flags & UNINTERESTING)
>>  			continue;
>>  
>> -		if (num_interesting++)
>> -			return error(_("last-modified can only operate on one tree at a time"));
>> +		if (num_interesting++) {
>> +			ret = error(_("last-modified can only operate on one revision at a time"));
>
> Do we maybe want to be a bit more specific and say committish instead of
> revision?

I was thinking about mentioning something like "commit-ish" instead, but I felt "commit-ish" isn't a commonly used term toward end-users. Looking at gitglossary(7), it says "revision" is a "synonym for commit". I'm happy to change this message, but I'm not sure s/revision/commit-ish/ is the best change for this.

Show 12 quoted lines

>> diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh
>> index 50f4312f71..1183ae667b 100755
>> --- a/t/t8020-last-modified.sh
>> +++ b/t/t8020-last-modified.sh
>> @@ -12,7 +12,7 @@ test_expect_success 'setup' '
>>  	test_commit 3 a/b/file
>>  '
>>  
>> -test_expect_success 'cannot run last-modified on two trees' '
>> +test_expect_success 'cannot run last-modified on two revision' '
>
> Nit: s/revision/revisions/

Thanks.

Show 6 quoted lines

>>  	test_must_fail git last-modified HEAD HEAD~1
>
> Another tiny nit: I'm always a bit wary around tests that don't verify
> the reason for failure. We might want to add:
>
>     test_grep "last-modified can only operate on one revision at a time" err

Okay, I'll rework this one and the one adding in the other commit.

-- 
Cheers,
Toon