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

[Eric Sunshine <sunshine@sunshineco.com>]

Thanks for the patch. Some comments below...

On Fri, Nov 10, 2017 at 11:32 AM, Robert P. J. Day
<rpjday@crashcourse.ca> 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 <rpjday@crashcourse.ca>
>
> ---
>
>   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 [(<rev>|<range>)...]
>   git bisect reset [<commit>]
> - 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<TAB>"  or
"...vi<TAB>" 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 [<commit>]
>         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.