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

[Michael Haggerty]

On 12/19/2016 07:23 PM, Junio C Hamano wrote:
> Michael Haggerty  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

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

[Junio C Hamano]

Michael Haggerty  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?

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

[Ian Jackson]

Michael Haggerty writes ("Re: [PATCH 0/5] git check-ref-format --stdin 
--report-errors"):
> Thanks for your patches. I left some comments about the individual patches.

Thanks for your review.

> I don't know whether this feature will be popular, but it's not a lot of
> code to add it, so it would be OK with me.

Great.

> 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`.

Sure.

> If it is thought likely that scripts will want to leave a pipe open to
> this command and feed it one query at a time, then it would be helpful
> to flush stdout after each reference's result is written. If the
> opposite use case is common (mass processing of refnames), we could
> always add a `--buffer` option like the one that `git cat-file --batch` has.

I think it should be unbuffered by default, so I will make that
change, along with the fixes from your other mails, and resubmit.

Regards,
Ian.

-- 
Ian Jackson <ijack...@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

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

[Michael Haggerty]

On 11/04/2016 08:13 PM, Ian Jackson wrote:
> I wanted to be able to syntax check lots of proposed refs quickly
> (please don't ask why - it's complicated!)
> 
> So I added a --stdin option to git-check-ref-format.  Also it has
> --report-errors now too so you can get some kind of useful error
> message if it complains.
> 
> It's still not really a good batch mode but it's good enough for my
> use case.  To improve it would involve a new command line option to
> offer a suitable stdout output format.
> 
> There are three small refactoring patches and the two patches with new
> options and corresponding docs.
> 
> Thanks for your attention.
> 
> FYI I am not likely to need this again in the near future: it's a
> one-off use case.  So my effort for rework is probably limited.  I
> thought I'd share what I'd done in what I hope is a useful form,
> anyway.

Thanks for your patches. I left some comments about the individual patches.

I don't know whether this feature will be popular, but it's not a lot of
code to add it, so it would be OK with me.

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`.

If it is thought likely that scripts will want to leave a pipe open to
this command and feed it one query at a time, then it would be helpful
to flush stdout after each reference's result is written. If the
opposite use case is common (mass processing of refnames), we could
always add a `--buffer` option like the one that `git cat-file --batch` has.

Michael

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

[Ian Jackson]

I wanted to be able to syntax check lots of proposed refs quickly
(please don't ask why - it's complicated!)

So I added a --stdin option to git-check-ref-format.  Also it has
--report-errors now too so you can get some kind of useful error
message if it complains.

It's still not really a good batch mode but it's good enough for my
use case.  To improve it would involve a new command line option to
offer a suitable stdout output format.

There are three small refactoring patches and the two patches with new
options and corresponding docs.

Thanks for your attention.

FYI I am not likely to need this again in the near future: it's a
one-off use case.  So my effort for rework is probably limited.  I
thought I'd share what I'd done in what I hope is a useful form,
anyway.

Regards,
Ian.