Re: [PATCH] bisect: mention "view" as an alternative to "visualize"

[Eric Sunshine]

On Fri, Nov 10, 2017 at 4:13 PM, Robert P. J. Day  wrote:
> On Fri, 10 Nov 2017, Eric Sunshine wrote:
>
>> Thanks for the patch. Some comments below...
>>
>> On Fri, Nov 10, 2017 at 11:32 AM, Robert P. J. Day
>>  wrote:
>> > Tweak a number of files to mention "view" as an alternative to
>> > "visualize":
>>
>> You probably want to end this sentence with a period, not a colon.
>
>   not sure about that, what's the standard when you're introducing a
> code snippet?

It wasn't clear that you were including a snippet since it's not
common practice to include the diffstat in the commit message on this
project...

>> >  Documentation/git-bisect.txt   | 9 -
>> >  Documentation/user-manual.txt  | 3 ++-
>> >  builtin/bisect--helper.c   | 2 +-
>> >  contrib/completion/git-completion.bash | 2 +-
>> >  git-bisect.sh  | 4 ++--
>> >  5 files changed, 10 insertions(+), 10 deletions(-)
>>
>> The diffstat belongs below the "---" separator, otherwise this text
>> will (undesirably) become part of the commit message proper.
>
>   no, i actually want that as part of the commit message. my standard
> is, if there are 5 or more files that get changed, i like to include a
> diff --stat in the commit message so people viewing the log can get a
> quick idea of how much has changed. if that's not desired here, i can
> remove it.

The same information is available to anyone interested in it simply by
asking for it:

git log --stat ...

More generally, commit messages should contain the really important
information you want to convey to the reader which _isn't_ available
some other way (by, for instance, taking advantage of the tool itself
-- such as the above --stat example).

On this project, the diffstat is never included as part of the commit
message, and it's likely that the patch won't be accepted by Junio
like that (or he'll just edit it out if he does accept it).

Thanks.

Re: [PATCH] bisect: mention "view" as an alternative to "visualize"

[Robert P. J. Day]

On Fri, 10 Nov 2017, Eric Sunshine wrote:

> Thanks for the patch. Some comments below...
>
> On Fri, Nov 10, 2017 at 11:32 AM, Robert P. J. Day
>  wrote:
> > Tweak a number of files to mention "view" as an alternative to
> > "visualize":
>
> You probably want to end this sentence with a period, not a colon.

  not sure about that, what's the standard when you're introducing a
code snippet?

> >  Documentation/git-bisect.txt   | 9 -
> >  Documentation/user-manual.txt  | 3 ++-
> >  builtin/bisect--helper.c   | 2 +-
> >  contrib/completion/git-completion.bash | 2 +-
> >  git-bisect.sh  | 4 ++--
> >  5 files changed, 10 insertions(+), 10 deletions(-)
>
> The diffstat belongs below the "---" separator, otherwise this text
> will (undesirably) become part of the commit message proper.

  no, i actually want that as part of the commit message. my standard
is, if there are 5 or more files that get changed, i like to include a
diff --stat in the commit message so people viewing the log can get a
quick idea of how much has changed. if that's not desired here, i can
remove it.

> > + git bisect visualize|view
>
> I think you need parentheses around these terms (see "git bisect
> skip", for example):
>
> git bisect (visualize | view)

  ah, quite so.

> However, in this case, it might be easier for readers if each is
> presented on its own line (and subsequent discussion can make it clear
> that they are synonyms).
>
> git bisect visualize
> git bisect view
>
> But, that's a matter of taste...

  given the precedent already established:

   git bisect (bad|new|) []
   git bisect (good|old|) [...]

i'll go with the parentheses and no intervening spaces. i'll let that
posting simmer for a bit longer before posting "v2".

rday

-- 


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

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

Re: [PATCH] bisect: mention "view" as an alternative to "visualize"

[Eric Sunshine]

Thanks for the patch. Some comments below...

On Fri, Nov 10, 2017 at 11:32 AM, Robert P. J. Day
 wrote:
> Tweak a number of files to mention "view" as an alternative to
> "visualize":

You probably want to end this sentence with a period, not a colon.

>  Documentation/git-bisect.txt   | 9 -
>  Documentation/user-manual.txt  | 3 ++-
>  builtin/bisect--helper.c   | 2 +-
>  contrib/completion/git-completion.bash | 2 +-
>  git-bisect.sh  | 4 ++--
>  5 files changed, 10 insertions(+), 10 deletions(-)

The diffstat belongs below the "---" separator, otherwise this text
will (undesirably) become part of the commit message proper.

> Signed-off-by: Robert P. J. Day 
>
> ---
>
>   here's hoping i have the right format for this patch ... are there
> any parts of this that are inappropriate, such as extending the bash
> completion?

This is the correct place for your commentary. The diffstat should
appear below your commentary.

> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> @@ -23,7 +23,7 @@ on the subcommand:
>   git bisect terms [--term-good | --term-bad]
>   git bisect skip [(|)...]
>   git bisect reset []
> - git bisect visualize
> + git bisect visualize|view

I think you need parentheses around these terms (see "git bisect
skip", for example):

git bisect (visualize | view)

However, in this case, it might be easier for readers if each is
presented on its own line (and subsequent discussion can make it clear
that they are synonyms).

git bisect visualize
git bisect view

But, that's a matter of taste...

> @@ -196,15 +196,14 @@ of `git bisect good` and `git bisect bad` to mark 
> commits.
>  Bisect visualize
>  
>
> -To see the currently remaining suspects in 'gitk', issue the following
> -command during the bisection process:
> +To see the currently remaining suspects in 'gitk', issue either of the
> +following equivalent commands during the bisection process:
>
>  
>  $ git bisect visualize
> +$ git bisect view
>  
>
> -`view` may also be used as a synonym for `visualize`.

Honestly, I think the original was clearer and placed a bit less
cognitive load on the reader. Moreover, for someone scanning the
documentation without reading it too deeply, the revised example makes
it seem as if it is necessary to invoke both commands rather than one
or the other.

> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> @@ -538,10 +538,11 @@ Note that the version which `git bisect` checks out for 
> you at each
>  point is just a suggestion, and you're free to try a different
>  version if you think it would be a good idea.  For example,
>  occasionally you may land on a commit that broke something unrelated;
> -run
> +run either of the equivalent commands
>
>  -
>  $ git bisect visualize
> +$ git bisect view
>  -

Same observation as above. This has the potential to confuse someone
quickly scanning the documentation into thinking that both commands
must be invoked. Merely stating in prose that one is the alias of the
other might be better.

>  which will run gitk and label the commit it chose with a marker that
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index fdd984d34..52f68c922 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1162,7 +1162,7 @@ _git_bisect ()
>  {
> __git_has_doubledash && return
>
> -   local subcommands="start bad good skip reset visualize replay log run"
> +   local subcommands="start bad good skip reset visualize view replay 
> log run"

People using muscle memory to type "git bisect v"  or
"...vi" might find it annoying for this to suddenly become
ambiguous. Just an observation; no strong opinion on it...

> diff --git a/git-bisect.sh b/git-bisect.sh
> @@ -20,7 +20,7 @@ git bisect next
> find next bisection to test and check it out.
>  git bisect reset []
> finish bisection search and go back to commit.
> -git bisect visualize
> +git bisect visualize|view
> show bisect status in gitk.

Again, this might be easier to read if split over two lines:

git bisect visualize
git bisect view
show bisect status in gitk.

in which case it's plenty clear that the commands are synonyms.

[PATCH] bisect: mention "view" as an alternative to "visualize"

[Robert P. J. Day]

Tweak a number of files to mention "view" as an alternative to
"visualize":

 Documentation/git-bisect.txt   | 9 -
 Documentation/user-manual.txt  | 3 ++-
 builtin/bisect--helper.c   | 2 +-
 contrib/completion/git-completion.bash | 2 +-
 git-bisect.sh  | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)

Signed-off-by: Robert P. J. Day 

---

  here's hoping i have the right format for this patch ... are there
any parts of this that are inappropriate, such as extending the bash
completion?

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 6c42abf07..89e6f9667 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -23,7 +23,7 @@ on the subcommand:
  git bisect terms [--term-good | --term-bad]
  git bisect skip [(|)...]
  git bisect reset []
- git bisect visualize
+ git bisect visualize|view
  git bisect replay 
  git bisect log
  git bisect run ...
@@ -196,15 +196,14 @@ of `git bisect good` and `git bisect bad` to mark commits.
 Bisect visualize
 

-To see the currently remaining suspects in 'gitk', issue the following
-command during the bisection process:
+To see the currently remaining suspects in 'gitk', issue either of the
+following equivalent commands during the bisection process:

 
 $ git bisect visualize
+$ git bisect view
 

-`view` may also be used as a synonym for `visualize`.
-
 If the `DISPLAY` environment variable is not set, 'git log' is used
 instead.  You can also give command-line options such as `-p` and
 `--stat`.
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 3a03e63eb..55ec58986 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -538,10 +538,11 @@ Note that the version which `git bisect` checks out for 
you at each
 point is just a suggestion, and you're free to try a different
 version if you think it would be a good idea.  For example,
 occasionally you may land on a commit that broke something unrelated;
-run
+run either of the equivalent commands

 -
 $ git bisect visualize
+$ git bisect view
 -

 which will run gitk and label the commit it chose with a marker that
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 35d2105f9..4b5fadcbe 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -46,7 +46,7 @@ static int check_term_format(const char *term, const char 
*orig_term)
return error(_("'%s' is not a valid term"), term);

if (one_of(term, "help", "start", "skip", "next", "reset",
-   "visualize", "replay", "log", "run", "terms", NULL))
+   "visualize", "view", "replay", "log", "run", "terms", 
NULL))
return error(_("can't use the builtin command '%s' as a term"), 
term);

/*
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index fdd984d34..52f68c922 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1162,7 +1162,7 @@ _git_bisect ()
 {
__git_has_doubledash && return

-   local subcommands="start bad good skip reset visualize replay log run"
+   local subcommands="start bad good skip reset visualize view replay log 
run"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__git_find_repo_path
diff --git a/git-bisect.sh b/git-bisect.sh
index 0138a8860..e8b622a47 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
 #!/bin/sh

-USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|view|replay|log|run]'
 LONG_USAGE='git bisect help
print this long help message.
 git bisect start [--term-{old,good}= --term-{new,bad}=]
@@ -20,7 +20,7 @@ git bisect next
find next bisection to test and check it out.
 git bisect reset []
finish bisection search and go back to commit.
-git bisect visualize
+git bisect visualize|view
show bisect status in gitk.
 git bisect replay 
replay bisection log.


-- 


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

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