threads / patch / 48634

v2t/perf/run: Use proper "--get-regexp", not "--get-regex"

Subject: [PATCH v2] t/perf/run: Use proper "--get-regexp", not "--get-regex"

## tl;dr

6 messages between Jun 3, 2018 and Jun 4, 2018. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Robert P. J. Day· Jun 3, 2018, 10:14 UTC · lore

Even though "--get-regex" appears to work with "git config", the clear standard is to spell out the action in full.

Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>

---

this is the only occurrence i saw of this in the entire code base, so it seemed worth tweaking just for consistency.

Show changes to t/perf/run +1 −1

diff --git a/t/perf/run b/t/perf/run
index 9aaa733c7..fb5753ea2 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -110,7 +110,7 @@ run_dirs () {
 get_subsections () {
 	section="$1"
 	test -z "$GIT_PERF_CONFIG_FILE" && return
-	git config -f "$GIT_PERF_CONFIG_FILE" --name-only --get-regex "$section\..*\.[^.]+" |
+	git config -f "$GIT_PERF_CONFIG_FILE" --name-only --get-regexp "$section\..*\.[^.]+" |
 	sed -e "s/$section\.\(.*\)\..*/\1/" | sort | uniq
 }

-- 
========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Thomas Gummerer· Jun 3, 2018, 13:30 UTC · re: Robert P. J. Day · lore

Re: [PATCH v2] t/perf/run: Use proper "--get-regexp", not "--get-regex"

> Subject: [PATCH v2] t/perf/run: Use proper "--get-regexp", not

micronit: we prefer starting with a lowercase letter after the "area:" prefix in commit messages. Junio can probably fix that while queuing, so no need to resend.

On 06/03, Robert P. J. Day wrote:

> 
> Even though "--get-regex" appears to work with "git config", the
> clear standard is to spell out the action in full.

--get-regex works as the parse-option API allows abbreviations of the full option to be specified as long as the abbreviation is unambiguos. I don't know if this is documented anywhere other than 'Documentation/technical/api-parse-options.txt' though.

> Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> 
> ---

It took me a bit to figure out why there is a v2, and what changed between the versions. This space after the '---' would be a good place to describe that to help reviewers.

For others that are curious, it seems like the word "clear" was added in the commit message.

The change itself looks good to me.

Show 26 quoted lines

> this is the only occurrence i saw of this in the entire code base, so
> it seemed worth tweaking just for consistency.
> 
> diff --git a/t/perf/run b/t/perf/run
> index 9aaa733c7..fb5753ea2 100755
> --- a/t/perf/run
> +++ b/t/perf/run
> @@ -110,7 +110,7 @@ run_dirs () {
>  get_subsections () {
>  	section="$1"
>  	test -z "$GIT_PERF_CONFIG_FILE" && return
> -	git config -f "$GIT_PERF_CONFIG_FILE" --name-only --get-regex "$section\..*\.[^.]+" |
> +	git config -f "$GIT_PERF_CONFIG_FILE" --name-only --get-regexp "$section\..*\.[^.]+" |
>  	sed -e "s/$section\.\(.*\)\..*/\1/" | sort | uniq
>  }
> 
> 
> -- 
> 
> ========================================================================
> Robert P. J. Day                                 Ottawa, Ontario, CANADA
>                   http://crashcourse.ca/dokuwiki
> 
> Twitter:                                       http://twitter.com/rpjday
> LinkedIn:                               http://ca.linkedin.com/in/rpjday
> ========================================================================

Robert P. J. Day· Jun 3, 2018, 13:36 UTC · re: Thomas Gummerer · lore

Re: [PATCH v2] t/perf/run: Use proper "--get-regexp", not "--get-regex"

On Sun, 3 Jun 2018, Thomas Gummerer wrote:

Show 5 quoted lines

> > Subject: [PATCH v2] t/perf/run: Use proper "--get-regexp", not
>
> micronit: we prefer starting with a lowercase letter after the "area:"
> prefix in commit messages.   Junio can probably fix that while
> queuing, so no need to resend.

  argh, i actually know that, i just screwed up.

Show 22 quoted lines

> On 06/03, Robert P. J. Day wrote:
> >
> > Even though "--get-regex" appears to work with "git config", the
> > clear standard is to spell out the action in full.
>
> --get-regex works as the parse-option API allows abbreviations of the
> full option to be specified as long as the abbreviation is
> unambiguos.  I don't know if this is documented anywhere other than
> 'Documentation/technical/api-parse-options.txt' though.
>
> > Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
> >
> > ---
>
> It took me a bit to figure out why there is a v2, and what changed
> between the versions.  This space after the '---' would be a good
> place to describe that to help reviewers.
>
> For others that are curious, it seems like the word "clear" was added
> in the commit message.
>
> The change itself looks good to me.

  the actual rationale for v2 was in the subject, i originally put
just "get-regex" rather then "--get-regex"; i resubmitted for
consistency.

rday

-- 
========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Philip Oakley· Jun 3, 2018, 15:33 UTC · re: Robert P. J. Day · lore

Re: [PATCH v2] t/perf/run: Use proper "--get-regexp", not "--get-regex"

From: "Robert P. J. Day" <rpjday@crashcourse.ca>

Show 19 quoted lines

> On Sun, 3 Jun 2018, Thomas Gummerer wrote:
>
>> > Subject: [PATCH v2] t/perf/run: Use proper "--get-regexp", not
>>
>> micronit: we prefer starting with a lowercase letter after the "area:"
>> prefix in commit messages.   Junio can probably fix that while
>> queuing, so no need to resend.
>
>  argh, i actually know that, i just screwed up.
>
>> On 06/03, Robert P. J. Day wrote:
>> >
>> > Even though "--get-regex" appears to work with "git config", the
>> > clear standard is to spell out the action in full.
>>
>> --get-regex works as the parse-option API allows abbreviations of the
>> full option to be specified as long as the abbreviation is
>> unambiguos.  I don't know if this is documented anywhere other than
>> 'Documentation/technical/api-parse-options.txt' though.

it's in `git help cli`:

many commands allow a long option --option to be abbreviated only to their unique prefix (e.g. if there is no other option whose name begins with opt, you may be able to spell --opt to invoke the --option flag), but you should fully spell them out when writing your scripts;

It's a worthwile read, even if the man page isn't flagged up that often.

Show 18 quoted lines

>>
>> > Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
>> >
>> > ---
>>
>> It took me a bit to figure out why there is a v2, and what changed
>> between the versions.  This space after the '---' would be a good
>> place to describe that to help reviewers.
>>
>> For others that are curious, it seems like the word "clear" was added
>> in the commit message.
>>
>> The change itself looks good to me.
>
>  the actual rationale for v2 was in the subject, i originally put
> just "get-regex" rather then "--get-regex"; i resubmitted for
> consistency.
>

-- Philip

Robert P. J. Day· Jun 4, 2018, 11:03 UTC · re: Philip Oakley · lore

Re: [PATCH v2] t/perf/run: Use proper "--get-regexp", not "--get-regex"

On Sun, 3 Jun 2018, Philip Oakley wrote:

> From: "Robert P. J. Day" <rpjday@crashcourse.ca>
> > On Sun, 3 Jun 2018, Thomas Gummerer wrote:

Show 15 quoted lines

> >> --get-regex works as the parse-option API allows abbreviations of
> >> the full option to be specified as long as the abbreviation is
> >> unambiguos.  I don't know if this is documented anywhere other
> >> than 'Documentation/technical/api-parse-options.txt' though.
>
> it's in `git help cli`:
>
> many commands allow a long option --option to be abbreviated only to
> their unique prefix (e.g. if there is no other option whose name
> begins with opt, you may be able to spell --opt to invoke the
> --option flag), but you should fully spell them out when writing
> your scripts;
>
> It's a worthwile read, even if the man page isn't flagged up that
> often.

  agreed that it's a good read and should be referenced more often.
one thing i don't see there, and it's based on an observation someone
once made (i believe on this list), is that even if there is
absolutely no ambiguity in a command, even if there are no pathspec
arguments, it's still worthwhile to add a trailing "--":

  $ git command options/treeish ... --

since that guarantees that git will waste no time trying to identify any ambiguity since you're being so precise. is that worth mentioning in that page?

rday

-- 
========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                  http://crashcourse.ca/dokuwiki

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

Junio C Hamano· Jun 4, 2018, 13:33 UTC · re: Robert P. J. Day · lore

Re: [PATCH v2] t/perf/run: Use proper "--get-regexp", not "--get-regex"

"Robert P. J. Day" <rpjday@crashcourse.ca> writes:

Show 10 quoted lines

> one thing i don't see there, and it's based on an observation someone
> once made (i believe on this list), is that even if there is
> absolutely no ambiguity in a command, even if there are no pathspec
> arguments, it's still worthwhile to add a trailing "--":
>
>   $ git command options/treeish ... --
>
> since that guarantees that git will waste no time trying to identify
> any ambiguity since you're being so precise. is that worth mentioning
> in that page?

I do not think it is worth mentioning _anywhere_ if you sell its benefit as "even there is no ambiguity it won't spend cycles".

The point of "git cmd X --" and "git cmd -- X" is that they save your human cycle, not machine cycle; you do not have to waste time wondering if you happen to have X as path in the working tree. That may be worth mentioning, but only "maybe" I would think.

A more important reason is you may not _know_ beforehand if X you mean to be a rev also happens to be a path (or vice versa) when you are scripting. Writing 'git checkout master', 'git diff HEAD', etc., in a script you intend to be generic enough is risky if 'master', HEAD, etc. can be both rev and path at the same time, but that is already described in gitcli page ;-)