doc: merge-tree: improve the script example

From: Kristoffer Haugsbakk <[email protected]>

From: Kristoffer Haugsbakk <[email protected]>

From: Kristoffer Haugsbakk <[email protected]>

From: Kristoffer Haugsbakk <[email protected]>

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 <[email protected]>

---

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(-)

Documentation/git-merge-tree.txt +0 -0

index 84cb2edf6d0..590cbf5df79 100644

Documentation/git-merge-tree.txt +0 -1

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

2.46.1.641.g54e7913fcb6

Documentation/git-merge-tree.txt used as a part of a series of steps such as: · line 216

Quoted 4 lines

        NEWTREE=$(git merge-tree --write-tree $BRANCH1 $BRANCH2)
        test $? -eq 0 || die "There were conflicts..."

Documentation/git-merge-tree.txt +1 -1

-       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 <[email protected]>

From: Kristoffer Haugsbakk <[email protected]>

This is easier to read.

Signed-off-by Kristoffer Haugsbakk <[email protected]>

---
  Documentation/git-merge-tree.txt | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

Documentation/git-merge-tree.txt +4 -4

index 590cbf5df79..10f8ac7f80a 100644
@@ -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

Quoted 2 lines · Kristoffer Haugsbakk

From: Kristoffer Haugsbakk <[email protected]>

Quoted 3 lines

From: Kristoffer Haugsbakk <[email protected]>

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 <[email protected]>

From: Kristoffer Haugsbakk <[email protected]>

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 <[email protected]>

---
 Documentation/git-merge-tree.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Documentation/git-merge-tree.txt +0 -0

index 10f8ac7f80a..46091da022d 100644

Quoted 5 lines · Kristoffer Haugsbakk

From: Kristoffer Haugsbakk <[email protected]>

From: Kristoffer Haugsbakk <[email protected]>

Quoted 4 lines

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 <[email protected]>

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 <[email protected]>

---

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?

Quoted 8 lines · Kristoffer Haugsbakk

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 <[email protected]>

---

Makes sense.

Documentation/git-merge-tree.txt used as a part of a series of steps such as: · line 218

Quoted 4 lines

Documentation/git-merge-tree.txt +2 -1

-       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?

Quoted 8 lines

Documentation/git-merge-tree.txt used as a part of a series of steps such as: · line 218

Quoted 3 lines

Documentation/git-merge-tree.txt +2 -0

+       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.

Quoted 7 lines

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.

Documentation/git-merge-tree.txt used as a part of a series of steps such as: · line 218

Quoted 4 lines

Documentation/git-merge-tree.txt +2 -1

-       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?

Documentation/git-merge-tree.txt linkgit:git-commit-tree[1], linkgit:git-write-tree[1], · line 217

Quoted 6 lines · Kristoffer Haugsbakk

Documentation/git-merge-tree.txt +4 -2

-       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:

Documentation/git-merge-tree.txt +4 -1

index 41dfb16476d..0b6a8a19b1f 100644
@@ -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 <[email protected]>

## 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 <[email protected]>

@@ 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

Quoted 2 lines · Kristoffer Haugsbakk

...
2.46.1.641.g54e7913fcb6

This round looks good to me; thanks.