Re: [PATCH] t1006: fix %(rest) test for object names with whitespace

[Junio C Hamano <gitster@pobox.com>]

Victoria Dye <vdye@github.com> writes:

>>> -    # FIXME: %(rest) is incompatible with object names that include whitespace,
>>> -    # e.g. HEAD:path/to/a/file with spaces. Use the resolved OID as input to
>>> -    # test this instead of the raw object name.
>>> -    if echo "$object_name" | grep -q " "; then
>>> -	test_rest=test_expect_failure
>>> -    else
>>> -	test_rest=test_expect_success
>>> -    fi
>>> -
>>> -    $test_rest '--batch-check with %(rest)' '
>>> +    # Use the resolved OID so %(rest) parsing is independent of whitespace
>>> +    # in object names (e.g. HEAD:path with spaces).
>>> +    test_expect_success '--batch-check with %(rest)' '
>>>   	echo "$type this is some extra content" >expect &&
>>> -	echo "$object_name    this is some extra content" |
>>> +	echo "$oid    this is some extra content" |
>> 
>> ... I somehow doubt that this is what 9fd38038 (t1006: update
>> 'run_tests' to test generic object specifiers, 2025-06-02) meant by
>> that comment.
>
> That FIXME was intended to call out the behavior of %(rest) in cat-file
> itself as something that we may eventually want to fix. The comment is
> only here because this test happens to demonstrate that behavior. For
> that reason, I'm also not sure I see the value of this patch; it's
> removing some visibility to a quirk of cat-file without fixing the
> underlying issue.

I agree that fixing underlying issue would be a much more valuable
outcome of resolving that FIXME comment, but isn't the approach to
give $object_name fundamentally incompatible with %(rest), making
the issue something %(rest) implementation cannot "fix", is it?

That is a part of the reason why I said I am dubious about the FIXME
comment in my comment.

Thanks.