On Dec 3, 2010, at 12:06 AM, Jonathan Nieder wrote:
> Hi,
>
> Kevin Ballard wrote:
>
>> [Subject: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful
>
> Maybe something like
>
> rebase-i: treat "edit" without sha1 as a request to amend previous commit
>
> would make the meaning more obvious in a shortlog.
That seems a bit misleading, though. This command really has nothing to do with
amending the previous commit. You can do anything you want once you break back to
the shell. I personally used it to run git-merge at that point in the history.
For this reason I'm a bit uneasy about overloading "edit", but it does have the
benefit that people already know "edit" brings them to the shell.
>> Extend the "edit" command to simply stop for editing if no sha1 is
>> given or if the sha1 is equal to "-". This behaves the same as "x false"
>> but is a bit friendlier for the user.
>
> Nice. I like the semantics.
>
>> * Picked up the extended explanation suggested by Jonathan Nieder.
>> I left off the last line about "noop" as that doesn't seem related.
>
> Right, please feel free to remind me if I forget to pick that up again.
>
>> * If the line given is "edit - some comments", emit "some comments" when
>> stopped. This is undocumented
>
> I think that's okay for now (though of course it would be best to explain
> some example uses in Documentation/git-rebase.txt in the form of examples).
Yep, I definitely need to add documentation.
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -469,12 +469,29 @@ do_next () {
>> + comment="$rest"
>> + line=$(git rev-list --pretty=oneline -1 --abbrev-commit --abbrev=7 HEAD)
>
> Hmm, the script seems to assume rev-list will not fail throughout. :/
> Ok.
>
>> + sha1="${line%% *}"
>> + rest="${line#* }"
>> + echo "$sha1" > "$DOTEST"/stopped-sha
>
> Maybe this can be done without relying on details of --pretty=oneline
> format?
>
> sha1=$(git rev-parse --short HEAD)
> rest=$(git show -s --format=%s HEAD)
Does this not similarly assume that rev-parse and show will not fail? Or was
the above comment only meant to point out this potential issue without
suggesting that it needed to be fixed?
> (Yes, elsewhere the script uses
>
> git rev-list --no-merges --pretty=oneline --abbrev-commit \
> --abbrev=7 --reverse --left-right --topo-order "$@" |
> sed -n "s/^>//p" |
> while read -r shortsha1 rest
>
> but in that loop, avoiding an extra exec seems more important.)
>
>> + fi
>> git rev-parse --verify HEAD > "$AMEND"
>> warn "Stopped at $sha1... $rest"
>> + if test -n "$comment"; then
>> + warn
>> + warn " $comment"
>> + warn
>
> Thanks, looks good to me.
>
> Ideas for tests? (see t3404 for inspiration)
I'll look into that. I wasn't really sure how to test this before, but t3404
does have some examples of testing the edit command already.
-Kevin Ballard--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html