Hi, I like the patch. Just a couple comments: On Fri, 20 Jun 2008, Stephan Beyer wrote: > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index e8ac2ae..aeb9628 100755 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -10,10 +10,25 @@ > # The original idea comes from Eric W. Biederman, in > # http://article.gmane.org/gmane.comp.version-control.git/22407 > > -USAGE='(--continue | --abort | --skip | [--preserve-merges] [--first-parent] > - [--preserve-tags] [--verbose] [--onto <branch>] <upstream> [<branch>])' > +OPTIONS_KEEPDASHDASH= > +OPTIONS_SPEC=' > +git-rebase -i [options] <upstream> [<branch>] > +git-rebase (--continue | --abort | --skip) Here, "-i" is not an error, but not required either, so it should be +git-rebase [-i] (--continue | --abort | --skip) > +-- > +continue Continue rebasing process > +abort Abort rebasing process and restore original branch > +skip Skip current patch and continue rebasing process I wonder if these options (which really should be subcommands) should not go below the options, for chronological reasons: you are likely to need the other options before --continue and friends. > +p,preserve-merges Try to recreate merges instead of ignoring s/$/ them/ > +t,preserve-tags Update tags to the new commit object > +m,merge Used anyways (no-op) > +i,interactive Used anyways (no-op) s/anyways/always/ Hmm? > +onto= Rebase onto given branch instead of upstream > +v,verbose Display a diffstat of what changed upstream > + When preserving merges: > +f,first-parent Show only commits following the first parent of each commit > +s,strategy= Use the given merge strategy > +' > > -OPTIONS_SPEC= > . git-sh-setup > require_work_tree > > @@ -595,32 +611,36 @@ do > > [...] > > + --onto) > + shift > + ONTO=$(git rev-parse --verify "$1") || > + die "Does not point to a valid commit: $1" > + ;; You probably need to introduce checks against "git rebase --continue --onto blabla", then. > ''|-h) > usage > ;; > - *) > + --) > + shift > + test $# -eq 1 -o $# -eq 2 || usage I consider this a bugfix. Thanks. > @@ -720,6 +731,7 @@ EOF > die_abort "Nothing to do" > > output git checkout $ONTO && do_rest > + exit 0 > ;; > esac > shift This exit is not really necessary, is it? Besides, a simple "exit" would be more correct: it reuses the exit status of the most recently executed command, which is what we want here. Thanks, Dscho -- 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
