Re: [PATCH 0/5] git check-ref-format --stdin --report-errors

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Dec 20, 2016, 07:29 UTC
Message-ID: <21317fc7-c1fb-8be0-eadf-90fed9486a48@alum.mit.edu>
In-Reply-To: <xmqqlgvbpyku.fsf@gitster.mtv.corp.google.com>

On 12/19/2016 07:23 PM, Junio C Hamano wrote:

Show 9 quoted lines

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> Especially given that the output is not especially machine-readable, it
>> might be more consistent with other commands to call the new feature
>> `--verbose` rather than `--report-errors`.
> 
> Don't we instead want to structure the output to be machine-readable
> instead, given that check-ref-format is a very low level plumbing
> command that is primarily meant for scriptors?

Of course that would be the ideal. Let's think about what it would look like. Given that the very purpose of the program is to decide whether its inputs are reasonable reference names or not, it would be important to make it bulletproof:

* It could be fed some ugly garbage
* It could be used for security-relevant checks

One obvious choice would be to use NUL separators, but that would make the output mostly unreadable to humans.

Another would be to use LF to terminate each line of output, like

    ok TAB refs/heads/foo LF
    bad TAB refs/heads/bad SP name@@.lock LF

For the LF-terminated `--stdin` input, this should be unambiguous. However, it wouldn't necessarily work for arguments passed in via the command line, for for slight variations on `--stdin` like if we were to add a `-z` option to allow the input to be NUL-terminated.

The 100% solution would probably be to support language-specific quoting, like the `--shell`/`--perl`/`--python`/`--tcl` options accepted by `for-each-ref`, probably with a fifth option for NUL-terminated output. And it should probably also support a `-z` option to make its input NUL-separated. Pretty much all of the infrastructure is already there in `quote.h` and `quote.c`, and the option-parsing could be cribbed from `builtin/for-each-ref.c`, so it wouldn't even be *that* much work to implement.

Michael