Re: [PATCH] git-rebase documentation: explain the exit code of custom commands

Stefan Beller [off-list ref] writes:

On Tue, Dec 30, 2014 at 9:33 AM, Junio C Hamano [off-list ref] wrote:

quoted

Stefan Beller [off-list ref] writes:

quoted

Signed-off-by: Stefan Beller <redacted>
---
 Documentation/git-rebase.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 924827d..ffadb0b 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt

@@ -372,7 +372,8 @@ idea unless you know what you are doing (see BUGS below).
 --exec <cmd>::
      Append "exec <cmd>" after each line creating a commit in the
      final history. <cmd> will be interpreted as one or more shell
-     commands.
+     commands. Rebasing will stop for manual inspection if the command
+     returns a non zero exit code.
 +
 This option can only be used with the `--interactive` option
 (see INTERACTIVE MODE below).

This is not wrong per-se, but I wonder if it belongs here, or
"INTERACTIVE MODE below" may be a better place for it.

I asked around in office yesterday for the --exec command and Jonathan
thought it was documented, but both him and me skimming over the man page
not find it. Maybe we stopped reading as we'd not expect it deep down
there any more?

Thanks for pointing out the place where it is documented actually.

quoted

In fact, I notice that "INTERACTIVE MODE below" already touches it,
like this:

    Reordering and editing commits usually creates untested intermediate
    steps.  You may want to check that your history editing did not break
    anything by running a test, or at least recompiling at intermediate
    points in history by using the "exec" command (shortcut "x").  You may
    do so by creating a todo list like this one:

    -------------------------------------------
    pick deadbee Implement feature XXX
    fixup f1a5c00 Fix to feature XXX
    exec make
    pick c0ffeee The oneline of the next commit
    edit deadbab The oneline of the commit after
    exec cd subdir; make test
    ...
    -------------------------------------------

    The interactive rebase will stop when a command fails (i.e. exits with
    non-0 status) to give you an opportunity to fix the problem. You can
    continue with `git rebase --continue`.

I further notice that this section mentions various insns you can
use in prose, but does not have a list of vocabulary.  A new user,
when using "rebase -i" for the first time, will only see a series of
"pick"s; there needs to be a short list of the available insns and
what each of them do somewhere in the documentation, organized in a
way similar to how command line options are listed and explained.

As a starting point, here is what I came up with.  I am not
committing this to anywhere in my tree yet---this is primarily for
discussion.

One thing that I find a bit troublesome is that we overuse the word
"command" to mean three different things.  In general, when we say
"The command does X" in a documentation for individual subcommand,
we mean that subcommand (i.e. "git rebase" in this case).  "Replace
the command 'pick' with Y" here however refers to an instruction
taken from the vocabulary the new list presents here with the word.

I like "instruction" being part of the vocabulary here.

quoted

And "exec stops when the command fails" refers to whatever shell
scriptlet you give 'exec' insn with the same word.  This (not just
the new text I am introducing, which follows the usage of the words
in the existing text, but the way we refer to "command" and mean
three different things in the entire text) needs to be cleaned up.



 Documentation/git-rebase.txt | 56 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 924827d..718300c 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt

@@ -509,20 +509,52 @@ By replacing the command "pick" with the

command "edit", you can tell
 the files and/or the commit message, amend the commit, and continue
 rebasing.

-If you just want to edit the commit message for a commit, replace the
-command "pick" with the command "reword".
-
+Here are the commands that can appear in the instruction sheet and
+their explanations.
+
+pick <commit> <subject>::
+       Replay the named commit on top of the current state.

I remember using git for the very first time (in 2010 to give you some
perspective)
and I had trouble with the rebase command in particular. If the
instruction sheet pops
up, there is a lot of text and as a novice user I was very unsure what
I was allowed to
touch, such that it doesn't break horribly.

In one of the moments I did not think properly through what I was doing,
I made the following change to the instruction sheet:

-pick deadbee Implement feature XXX
+pick deadbee file-xyz.c: implement feature XXX/

Notice I did not change the instruction nor the "named
commit"(deadbee) as you put it.
I tried to rename the subject line of the commit in place. This did
not work of course, but still
picked up the commit the way it was.

I am telling this story now as I am a bit troubled with "named
commit". It actually means
the commit with the given sha1 value (side question: Could I also use
the full git language to
describe commits like: "pick topicbranch^2~3" ?) The text after the
abbreviated sha1 seems
only commentary to me.

So as another thought we may want to introduce a "reword subject
line", which replaces the
subject line of the "named commit" by the comment in the rest of the
line of the instruction sheet.

quoted

+
+edit <commit> <subject>::
+       Replay the named commit and then stop, so that the user can
+       edit the working tree files and "git rebase --continue" to
+       record the modified changes made to the working tree files
+       while tweaking the log message.

This is not complete. If I edit the working tree files and then "git
rebase --continue",
I am told
    Documentation/SubmittingPatches: needs update
    You must edit all merge conflicts and then
    mark them as resolved using git add

The way I see the edit instruction is more like a "stop", so mentally
I translate
     pick c0ffeee The oneline of the next commit
     edit deadbab The oneline of the commit after
     pick c0ffaaa Another commit
to:
     pick c0ffeee The oneline of the next commit
     pick deadbab The oneline of the commit after
     stop # change files and amend changes to the previous commit or
hack&commit new stuff
     pick c0ffaaa Another commit

How about:
    Replay the named commit and then stop, such that the user can
    modify the commit by ammending changes or add new commits in between.

quoted

+
+reword <commit> <subject>::
+       Replay the named commit and then open the editor to tweak
+       the log message (i.e. without modifying the changes made to
+       the working tree contents).
+
+squash <commit> <subject>::
+       Modify the current commit by squashing the change made by
+       the named commit, and then open the editor to modify the log
+       message using existing messages from the current commit and
+       the named commit.  The authorship of the resulting commit is
+       taken from the current commit.
+       See `--autosquash`.

Should this be s/current/previous/. Technically the current commit seems
correct to me,...

My primary focus was to suggest a way to revamp the overall
structure of the documentation for a better organization, and didn't
think about details of word choice in individual sentences; I wasn't
meaning to come up with the final wording anyway.

Overall I like that change as it would help finding the behavior for
exec easier.

Yeah, as long as we can agree that it would make the result easier
to read to revamp the overall structure to have a list of insns and
their description in an enumerated list, I'd be happy ;-).