Junio C Hamano <gitster@xxxxxxxxx> writes:
> Dirk Gouders <dirk@xxxxxxxxxxx> writes:
>
>> 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"
>>>> ...
>>> 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.
>
> I cannot tell if you really meant the double negative involving
> "doubt", but assuming you did, you are saying that
I'm sorry about that double negative which was probably wrong wording of
a non-native speaker.
> With "X is not Y", it is clear enough that we expect X to be Y
> (if it were not clear to somebody who read "X is not Y" that we
> want X to be Y, then "X is not Y, but it should be" may needed,
> but "X is not Y" is clear enough).
>
> So you think "$1 is either missing or empty" is better without "but
> should not be" added to the end? Am I reading you correctly?
>
> I think this takes us back pretty much to square one ;-) but that is
> also fine.
>
> But the above argument depends on an untold assumption. The message
> "X is not Y" must be clearly understood as a complaint, not a mere
> statement of a fact. I am not sure if that is the case.
>
> Instead of "X is not Y, but it should be", the way to clarify these
> messages may be to say "error: X is not Y", perhaps?
That is exactly what came to my mind when I was later re-thinking
what I had written.
>> 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
>
> If you are worried about "test -s" failing because "$last_arg" does
> not exist, then you are worried too much. We upfront guard the
> test_grep() helper with "test -f" of the same file and diagnoses the
> lack of the file as a bug in the test. And we do not assume gremlins
> removing random files while we are running tests.
Yes, thank you for clarification and sorry for the noise.
Dirk
