Re: [PATCH 2/3] vimdiff: add tool documentation

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 8, 2021, 01:02 UTC
Message-ID: <CAPig+cSi9S+EdqKLNWDGA76B3kUbvxQHEh2csyZYqX2sf0gztQ@mail.gmail.com>
In-Reply-To: <CAJDDKr4icrigRO-2S_HVdY7+W0EGJ8XBn5dD=O-qy1tXym4u8w@mail.gmail.com>

On Sun, Nov 7, 2021 at 4:25 PM David Aguilar <davvid@gmail.com> wrote:

Show 14 quoted lines

> On Thu, Nov 4, 2021 at 9:10 AM Fernando Ramos <greenfoo@u92.eu> wrote:
> > +                               while IFS= read -r line
> > +                               do
> > +                                       printf "%s\t%s\n" "$per_line_prefix" "$line"
> > +                                done < <(diff_mode && diff_cmd_help "$toolname" || merge_cmd_help "$toolname")
>
> If we wanted to shorten this line a bit, would it work to run the pipeline
> first and pipe the result?
>
>     (diff_mode && ... || merge_cmd_help ...) |
>     while IFS= read -r line
>     do
>        ...
>     done

The additional benefit is that this avoids the `<(...)` Bashism (which you mentioned in your other review).

Show 10 quoted lines

> > +               cat <<-ENDOFMESSAGE
> > +                       Opens vim with two vertical windows: LOCAL changes will be placed in the left
> > +                       window and REMOTE changes in the right one.
> > +               ENDOFMESSAGE
>
> Tiny nit: we call this EOF in the other places
> (git-mergetool--helper.sh) where we do the same.
>
> ENDOFMESSAGE is a bit verbose so it might be worth sticking to the
> conventional EOF marker.

A couple additional really micro nits (take them or leave them)...

We normally don't add extra indentation to the here-doc body, and we use `<<-\EOF` to reduce cognitive load if there's nothing in the body which requires interpolation or expansion. Thus:

    cat <<-\EOF
    Opens vim with two...
    EOF