Re: [PATCH 1/2] t7508: demonstrate status's failure to use --porcelain format with -z

From: Brandon Casey <casey@nrlssc.navy.mil>
Date: May 26, 2011, 22:51 UTC
Message-ID: <oxsohkXWsdC5MeWOTaXZMEPAVeHglxeXSUXEWJ6xA_rmLcm2iqjtpg@cipher.nrlssc.navy.mil>
In-Reply-To: <7v4o4hw2zm.fsf@alter.siamese.dyndns.org>

On 05/26/2011 04:45 PM, Junio C Hamano wrote:

Show 14 quoted lines

> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
>> From: Brandon Casey <drafnel@gmail.com>
>>
>> When 'git status' is supplied the -z switch, and no output format has been
>> selected, it is supposed to use the --porcelain format.  This does not
>> happen.  Instead, the standard long format is used.  Add a test to
>> demonstrate this failure.
>>
>> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> 
> I didn't even know we attempted to default to porcelain when -z is given,
> even though it is a logical thing to do in the sense that nobody sane
> would want to read a NUL-terminated human readable format.

The behavior is derived from your original behavior where -z was supposed to imply --short (see 173e6c8). You even documented it: 9e4b7ab :) Later, Jeff added --porcelain (6f15787), and updated the documentation:

   -z::
  -       Terminate entries with NUL, instead of LF.  This implies `-s`
  -       (short status) output format.
  +       Terminate entries with NUL, instead of LF.  This implies
  +       the `--porcelain` output format if no other format is given.

Show 6 quoted lines

> I'll rewrite the test to avoid hardcoded and context dependent
> test vector, like this:
> 
>     test_expect_failure 'status -z implies porcelain' '
>            git status --porcelain |
>            perl -pe "s/\012/\000/g" >expect

Missing '&&' here.

Show 6 quoted lines

>            git status -z >output &&
>            test_cmp expect output
>     '
> 
> as the only thing you are interested in is the two output to match
> identically modulo the record termination.

Thanks.

> By the way, don't we however also want to make sure -z does not kick in
> automatically when other options like "short" or "normal" is given?

It seems it was intended to be able to do

   git status -z --short

and since -z implies --porcelain if --short is not given, it is not possible to produce "normal" long format and NUL termination at the same time.

So, building on your modified test above, we could add something like:

     test_expect_failure 'status -z with -s works correctly' '
            git status -s |
            perl -pe "s/\012/\000/g" >expect &&
            git status -s -z >output &&
            test_cmp expect output
     '

Hopefully, the existing tests for status, without -z, will notice if NUL termination kicks in when it is not supposed to.

-Brandon