patch, 3 partsdoc: merge-tree: improve the script example

>
> From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
>
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> Provide a commit message in the example command.
>
> The command will hang since it is waiting for a commit message on
> stdin.  Which is usable but not straightforward enough since this is
> example code.

>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
>
> Notes (series):
>     Unlike on some other manuals you probably won’t end up running these
>     commands directly to test things out.  But you might end up copying and
>     modifying it when playing around with the command.
>
>  Documentation/git-merge-tree.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
> index 84cb2edf6d0..590cbf5df79 100644
> --- a/Documentation/git-merge-tree.txt
> +++ b/Documentation/git-merge-tree.txt
> @@ -213,7 +213,7 @@ used as a part of a series of steps such as:
>
>         NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2)
>         test $? -eq 0 || die "There were conflicts..."
> -       NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)
> +       NEWCOMMIT=$(git commit-tree $NEWTREE -mMerge -p $BRANCH1 -p $BRANCH2)

>         git update-ref $BRANCH1 $NEWCOMMIT
>
>  Note that when the exit status is non-zero, `NEWTREE` in this sequence
> --
> 2.46.1.641.g54e7913fcb6

>>         NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2)
>>         test $? -eq 0 || die "There were conflicts..."
>> -       NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)
>> +       NEWCOMMIT=$(git commit-tree $NEWTREE -mMerge -p $BRANCH1 -p $BRANCH2)
>
> ...perhaps this can be -F ${FILE_WITH_COMMIT_MESSAGE} ?  I personally
> have a problem with writing example code that models horrible commit
> messages; I'd rather give them an example that hangs waiting on stdin
> than do that.

> From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
> 
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> 
> This is easier to read.

> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
>   Documentation/git-merge-tree.txt | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
> index 590cbf5df79..10f8ac7f80a 100644
> --- a/Documentation/git-merge-tree.txt
> +++ b/Documentation/git-merge-tree.txt
> @@ -211,12 +211,12 @@ linkgit:git-commit-tree[1], linkgit:git-write-tree[1],
>   linkgit:git-update-ref[1], and linkgit:git-mktag[1].  Thus, it can be
>   used as a part of a series of steps such as:
>   
> -       NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2)
> +       newtree=$(git merge-tree --write-tree $branch1 $branch2)
>          test $? -eq 0 || die "There were conflicts..."
> -       NEWCOMMIT=$(git commit-tree $NEWTREE -mMerge -p $BRANCH1 -p $BRANCH2)
> -       git update-ref $BRANCH1 $NEWCOMMIT
> +       newcommit=$(git commit-tree $newtree -mMerge -p $branch1 -p $branch2)
> +       git update-ref $branch1 $newcommit
>   
> -Note that when the exit status is non-zero, `NEWTREE` in this sequence
> +Note that when the exit status is non-zero, `newtree` in this sequence
>   will contain a lot more output than just a tree.
>   
>   For conflicts, the output includes the same information that you'd get

>
> Hi Kristoffer
>
> On 07/10/2024 12:10, Kristoffer Haugsbakk wrote:
> > From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
> >
> > From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> >
> > This is easier to read.
>
> I think that is a matter of taste, one could argue that using uppercase
> variable names makes them stand out so the user can see which arguments
> are parameters more clearly.

>
> From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
>
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> Use `||` directly since that is more straightforward than checking the
> last exit status.
>
> Also use `echo` and `exit` since `die` is not defined.

> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
>  Documentation/git-merge-tree.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
> index 10f8ac7f80a..46091da022d 100644
> --- a/Documentation/git-merge-tree.txt
> +++ b/Documentation/git-merge-tree.txt
> @@ -211,8 +211,10 @@ linkgit:git-commit-tree[1], linkgit:git-write-tree[1],
>  linkgit:git-update-ref[1], and linkgit:git-mktag[1].  Thus, it can be
>  used as a part of a series of steps such as:
>
> -       newtree=$(git merge-tree --write-tree $branch1 $branch2)
> -       test $? -eq 0 || die "There were conflicts..."
> +       newtree=$(git merge-tree --write-tree $branch1 $branch2) || {
> +           echo "There were conflicts..." 1>&2
> +           exit 1
> +       }

> On Mon, Oct 7, 2024 at 4:11 AM Kristoffer Haugsbakk
> <code@khaugsbakk.name> wrote:
>>
>> From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
>>
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>
>> Use `||` directly since that is more straightforward than checking the
>> last exit status.
>>
>> Also use `echo` and `exit` since `die` is not defined.
>
> Not defined where?  It's defined in my scripts...  ;-)
>
> Just kidding; this is a good change.

> From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
> 
> 
> Kristoffer Haugsbakk (3):
>    doc: merge-tree: provide a commit message
>    doc: merge-tree: use lower-case variables
>    doc: merge-tree: use || directly
> 
>   Documentation/git-merge-tree.txt | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> --
> 2.46.1.641.g54e7913fcb6
> 

> Hi Kristoffer
>
> Thanks for working on this. Patches 1 and 3 look like clear improvements 
> to me, I've left a comment on patch 2 as the improvement is less clear 
> to me but I wouldn't object to it being merged.
>
> Best Wishes
>
> Phillip

> Provide a commit message in the example command.
>
> The command will hang since it is waiting for a commit message on
> stdin.  Which is usable but not straightforward enough since this is
> example code.
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---

> Kristoffer Haugsbakk <code@khaugsbakk.name> writes:
>
>> Provide a commit message in the example command.
>>
>> The command will hang since it is waiting for a commit message on
>> stdin.  Which is usable but not straightforward enough since this is
>> example code.
>>
>> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>> ---
>
> Makes sense.
>
>> -       NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)
>> +       NEWCOMMIT=$(git commit-tree $NEWTREE -F $FILE_WITH_COMMIT_MESSAGE \
>> +           -p $BRANCH1 -p $BRANCH2)
>>         git update-ref $BRANCH1 $NEWCOMMIT
>
> The shell should know, after seeing $FILE_WITH_COMMIT_MESSAGE and
> encountering the end of line, that you haven't completed telling
> what you started telling it.  Do you need " \" at the end of the
> line?

> I know that it was suggested to use a file with message, and I agree
> with the suggestion, but then I wonder if we want to be more
> complete and show that a file gets prepared in the example to avoid
> making readers wonder where $FILE_WITH_COMMIT_MESSAGE comes from?
>
> E.g.,
>
>         vi message.txt
>         NEWCOMMIT=$(git comimt-tree $NEWTREE -F message.txt
>                     -p $BRANCH1 -p $BRANCH2)
>
> or something like that?

>>> +       NEWCOMMIT=$(git commit-tree $NEWTREE -F $FILE_WITH_COMMIT_MESSAGE \
>>> +           -p $BRANCH1 -p $BRANCH2)
>>>         git update-ref $BRANCH1 $NEWCOMMIT
>>
>> The shell should know, after seeing $FILE_WITH_COMMIT_MESSAGE and
>> encountering the end of line, that you haven't completed telling
>> what you started telling it.  Do you need " \" at the end of the
>> line?
>
> I tried that and got an error: `-p: not found`.

>> E.g.,
>>
>>         vi message.txt
>>         NEWCOMMIT=$(git comimt-tree $NEWTREE -F message.txt
>>                     -p $BRANCH1 -p $BRANCH2)
>>
>> or something like that?
>
> I’ll do that.

>> -       NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)
>> +       NEWCOMMIT=$(git commit-tree $NEWTREE -F $FILE_WITH_COMMIT_MESSAGE \
>> +           -p $BRANCH1 -p $BRANCH2)
>>         git update-ref $BRANCH1 $NEWCOMMIT
>
> The shell should know, after seeing $FILE_WITH_COMMIT_MESSAGE and
> encountering the end of line, that you haven't completed telling
> what you started telling it.  Do you need " \" at the end of the
> line?

> -       NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2)
> -       test $? -eq 0 || die "There were conflicts..."
> +       NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2) || {
> +           echo "There were conflicts..." 1>&2
> +           exit 1
> +       }

> Kristoffer Haugsbakk <code@khaugsbakk.name> writes:
>
>> -       NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2)
>> -       test $? -eq 0 || die "There were conflicts..."
>> +       NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2) || {
>> +           echo "There were conflicts..." 1>&2
>> +           exit 1
>> +       }
>
> Makes sense.  Was there a particular reason why these two patches
> had to be done in two separate steps?
>
> Looking good otherwise.  Thanks.

>
> § Changes since v2
>
> • Squash into one patch
> • Use `vi message.txt` before `git commit-tree`
> • Also declare the branches
>   • Fully qualified for `BRANCH1` since it is passed to
>     git-update-ref(1)
>
> Kristoffer Haugsbakk (1):
>   doc: merge-tree: improve example script
>
>  Documentation/git-merge-tree.txt | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> Interdiff against v2:
> diff --git a/Documentation/git-merge-tree.txt b/Documentation/git-merge-tree.txt
> index 41dfb16476d..0b6a8a19b1f 100644
> --- a/Documentation/git-merge-tree.txt
> +++ b/Documentation/git-merge-tree.txt
> @@ -211,11 +211,14 @@ linkgit:git-commit-tree[1], linkgit:git-write-tree[1],
>  linkgit:git-update-ref[1], and linkgit:git-mktag[1].  Thus, it can be
>  used as a part of a series of steps such as:
>
> +       vi message.txt
> +       BRANCH1=refs/heads/test
> +       BRANCH2=main
>         NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2) || {
>             echo "There were conflicts..." 1>&2
>             exit 1
>         }
> -       NEWCOMMIT=$(git commit-tree $NEWTREE -F $FILE_WITH_COMMIT_MESSAGE \
> +       NEWCOMMIT=$(git commit-tree $NEWTREE -F message.txt \
>             -p $BRANCH1 -p $BRANCH2)
>         git update-ref $BRANCH1 $NEWCOMMIT
>
> Range-diff against v2:
> 1:  b1ca5cae768 < -:  ----------- doc: merge-tree: provide a commit message
> 2:  13e0f3bed5d ! 1:  c7e0d76e71e doc: merge-tree: use || directly
>     @@ Metadata
>      Author: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
>       ## Commit message ##
>     -    doc: merge-tree: use || directly
>     +    doc: merge-tree: improve example script
>
>     -    Use `||` directly since that is more straightforward than checking the
>     -    last exit status.
>     +    • Provide a commit message in the example command.
>
>     -    Also use `echo` and `exit` since `die` is not defined.
>     +      The command will hang since it is waiting for a commit message on
>     +      stdin.  Which is usable but not straightforward enough since this is
>     +      example code.
>     +    • Use `||` directly since that is more straightforward than checking the
>     +      last exit status.
>     +
>     +      Also use `echo` and `exit` since `die` is not defined.
>     +    • Expose variable declarations.
>
>          Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
>     @@ Documentation/git-merge-tree.txt: linkgit:git-commit-tree[1], linkgit:git-write-
>
>      -       NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2)
>      -       test $? -eq 0 || die "There were conflicts..."
>     +-       NEWCOMMIT=$(git commit-tree $NEWTREE -p $BRANCH1 -p $BRANCH2)
>     ++       vi message.txt
>     ++       BRANCH1=refs/heads/test
>     ++       BRANCH2=main
>      +       NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2) || {
>      +           echo "There were conflicts..." 1>&2
>      +           exit 1
>      +       }
>     -        NEWCOMMIT=$(git commit-tree $NEWTREE -F $FILE_WITH_COMMIT_MESSAGE \
>     -            -p $BRANCH1 -p $BRANCH2)
>     ++       NEWCOMMIT=$(git commit-tree $NEWTREE -F message.txt \
>     ++           -p $BRANCH1 -p $BRANCH2)
>              git update-ref $BRANCH1 $NEWCOMMIT
>     +
>     + Note that when the exit status is non-zero, `NEWTREE` in this sequence
> --
> 2.46.1.641.g54e7913fcb6

> On Wed, Oct 9, 2024 at 9:54 AM Kristoffer Haugsbakk
> <code@khaugsbakk.name> wrote:
>> ...
>> 2.46.1.641.g54e7913fcb6
>
> This round looks good to me; thanks.