Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> On Fri, Mar 1, 2024 at 5:11 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Eric Sunshine <ericsunshine@xxxxxxxxxxx> writes:
>> > A more accurate message might be "'foo' is empty but
>> > should not be (or doesn't exist)", but that's unnecessarily long-winded
>> > and adds little information that the test author couldn't discover by
>> > noticing the file's absence.
>>
>> The "adds little information" version may be
>>
>> echo "'$1' is either missing or empty, but should not be"
>>
>> And avoiding "X is Y, but should be ~Y" construct, perhaps
>>
>> echo "'$1' should be a file with non-empty contents"
>>
>> would work better? I dunno.
>
> I find "'$1' is either missing or empty, but should not be" suggestion
> clear and easily understood. I'll reroll with that.
This is a view from a position with more distance:
I find that not so easily understood -- the "but should not
be" part is rather unexpected and I feel, it doesn't provide necessary
information, e.g.:
test_path_is_executable () {
...
echo "$1 is not executable"
...
also doesn't state what is wanted and I doubt that message doesn't
clearly describe the problem.
While I looked at it: there is another `test -s` in test_grep () that
perhaps could be fixed the same way:
if test -s "$last_arg"
then
cat >&4 "$last_arg"
else
echo >&4 "<File '$last_arg' is empty>"
fi
Dirk
