[PATCH] show-ref: place angle brackets around variables in usage string

[PATCH] show-ref: place angle brackets around variables in usage string

[Alex Henrie]

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/show-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index dfbc314..131ef28 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -8,7 +8,7 @@
 
 static const char * const show_ref_usage[] = {
 	N_("git show-ref [-q | --quiet] [--verify] [--head] [-d | --dereference] [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags] [--heads] [--] [<pattern>...]"),
-	N_("git show-ref --exclude-existing[=pattern] < ref-list"),
+	N_("git show-ref --exclude-existing[=<pattern>] < <ref-list>"),
 	NULL
 };
 
-- 
2.5.0

Re: [PATCH] show-ref: place angle brackets around variables in usage string

[Philip Oakley]

From: "Alex Henrie" <alexhenrie24@gmail.com>
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
> builtin/show-ref.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/show-ref.c b/builtin/show-ref.c
> index dfbc314..131ef28 100644
> --- a/builtin/show-ref.c
> +++ b/builtin/show-ref.c
> @@ -8,7 +8,7 @@
>
> static const char * const show_ref_usage[] = {
>  N_("git show-ref [-q | --quiet] [--verify] [--head] [-d 
> | --dereference] [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags] 
> [--heads] [--] [<pattern>...]"),
> - N_("git show-ref --exclude-existing[=pattern] < ref-list"),
> + N_("git show-ref --exclude-existing[=<pattern>] < <ref-list>"),

Should the '<' stdin redirection be shown?

It looks (at first glance) as if this gained a double '< <' at the 
beginning of 'ref-list', rather than being a clean indication of the 
redirection. Perhaps change 'ref-list' to 'ref-list-file' for a slight 
improvement in clarity - this it's only occurance, and the redirection 
would best match a file.


>  NULL
> };
>
> -- 
> 2.5.0
>
--
Philip
(will be offline for 4 days) 

Re: [PATCH] show-ref: place angle brackets around variables in usage string

[Alex Henrie]

2015-08-29 4:21 GMT-06:00 Philip Oakley <philipoakley@iee.org>:
> Should the '<' stdin redirection be shown?
>
> It looks (at first glance) as if this gained a double '< <' at the beginning
> of 'ref-list', rather than being a clean indication of the redirection.
> Perhaps change 'ref-list' to 'ref-list-file' for a slight improvement in
> clarity - this it's only occurance, and the redirection would best match a
> file.

This syntax occurs in three other places in Git:

git cat-file (--batch | --batch-check) [--follow-symlinks] < <list-of-objects>

git check-attr --stdin [-z] [-a | --all | <attr>...] < <list-of-paths>

git hash-object  --stdin-paths < <list-of-paths>

So if we need to say <ref-list-file> for clarity, we should also say
<object-list-file> and <path-list-file> for these other commands.

I think the most sane thing to do is to commit this patch as-is, and
then someone can submit a separate patch to reword all four usage
strings for increased clarity.

-Alex

Re: [PATCH] show-ref: place angle brackets around variables in usage string

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Alex Henrie" <alexhenrie24@gmail.com>
>> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
>> ---
>> builtin/show-ref.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/builtin/show-ref.c b/builtin/show-ref.c
>> index dfbc314..131ef28 100644
>> --- a/builtin/show-ref.c
>> +++ b/builtin/show-ref.c
>> @@ -8,7 +8,7 @@
>>
>> static const char * const show_ref_usage[] = {
>>  N_("git show-ref [-q | --quiet] [--verify] [--head] [-d 
>> | --dereference] [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]
>> [--heads] [--] [<pattern>...]"),
>> - N_("git show-ref --exclude-existing[=pattern] < ref-list"),
>> + N_("git show-ref --exclude-existing[=<pattern>] < <ref-list>"),
>
> Should the '<' stdin redirection be shown?

Hmm, that actually is an interesting thought, because commands that
take their input from the standard input in practice would take the
input from upstream pipe more often than from a static file on the
filesystem, i.e.

    $ <cmd> | git show-ref --exclude-existing[=<pattern>]

would be more common, and the "input from file" would more likely
than not follow this pattern anyway:

    $ <cmd> > <file>
    $ git show-ref --exclude-existing[=<pattern>] < <file>

A quick "git grep -e ' < ' Documentation/" tells me that there
aren't that many ones that take input from stdin.

I am wondering if we can take this one as an example that is among
the cleaner and easier to understand:

(GOOD)  'git stripspace' [-s | --strip-comments] < input

and extend its idea further.  What is happening here is that "< input"
gives rather a clear sign that it is not saying that the user must
name her input file "input" --- any intelligent user can substitute it
with the name of the file she has without being told with a noisy
and unsightly

(BAD)   'git stripspace' [-s | --strip-comments] < <input>

mostly because "input" is so a generic word already.  Perhaps we
could even drop "< anything" as you suggest.

> It looks (at first glance) as if this gained a double '< <' at the
> beginning of 'ref-list', rather than being a clean indication of the
> redirection. Perhaps change 'ref-list' to 'ref-list-file' for a slight
> improvement in clarity - this it's only occurance, and the redirection
> would best match a file.

Or replace it with "< input", together with other ones.  Especially
the synopsys that says "git $cmd --stdin < <$anything>" can be
replaced with "git $cmd --stdin" without losing any clarity.

The only thing that it loses is "what is fed from the standard input
is a list of refs", but that should be left to the description of
the option that introduces this "read from the standard input"
behaviour to the command.  "git $cmd --help" for rev-list and
update-index may serve as examples; they do not have this ugly
"< <input>" business.

Hmm?

Re: [PATCH] show-ref: place angle brackets around variables in usage string

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>>> - N_("git show-ref --exclude-existing[=pattern] < ref-list"),
>>> + N_("git show-ref --exclude-existing[=<pattern>] < <ref-list>"),
>>
>> Should the '<' stdin redirection be shown?
>
> Hmm, that actually is an interesting thought,...

Having said all that (i.e. I agree dropping "< $anything" may be a
good idea in general), for this particular command, I suspect that
the feature may have outlived its usefulness.  It was introduced
(silently) in ed9f7c95 (git-fetch: Avoid reading packed refs over
and over again, 2006-12-17) as an optimisation that is very specific
to the way how "git fetch" scripted Porcelain happened to be written
back then.

The way the feature is misdesigned clearly shows that it was not
designed for general consumption---if it were, we would have made
"show-ref" to be usable as a filter even when we are not excluding
anything with "--exclude-existing" option.

Of course, it is OK and definitely a lot safer to keep the "can work
as a filter" feature for possible third-party uses, but if we were
to go that route, it is not too late to extend the machinery so that
"work as a filter" is not tied too closely to "exclude-existing".