Thread (5 messages) flat view 5 messages, 2 authors, 2016-06-15

[PATCHv2 0/2] Add a "fixup" command to "rebase --interactive"

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:47:50

Thanks, everybody, for all of the feedback.  This is the redone patch
series, which I think addresses all of your suggestions.

I would still like to implement "don't open the editor if there are
only fixups", but if it's OK I'll work on that in a separate patch
series when I have time.

Michael J Gruber wrote:
My first reaction to the subject was "Huh? What repository?". So I
suggest something like

t3404: Better document the original repository layout

as a more descriptive subject.
Good idea.  Done.

Johannes Schindelin wrote:
Alternatively, one could use the test_commit function, I guess. [...]
Yes, that's much easier.  Changed.  This makes the old first and
second patches reduce to a single one.

Johannes Schindelin wrote:
On Fri, 4 Dec 2009, Junio C Hamano wrote:
quoted
Michael Haggerty [off-list ref] writes:
quoted
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0bd3bf7..539413d 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -302,7 +302,7 @@ nth_string () {
 
 make_squash_message () {
 	if test -f "$SQUASH_MSG"; then
-		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
+		COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)\(th\|st\|nd\|rd\) commit message.*:/\1/p" \
 			< "$SQUASH_MSG" | sed -ne '$p')+1))
This sed replacement worries me.  I don't have a time to check myself
today but do we use \(this\|or\|that\) alternates with our sed script
already elsewhere in the codebase (test scripts do not count)?

Otherwise this may suddenly be breaking a platform that has an
implementation of sed that may be substandard but so far has been
sufficient to work with git.
IIRC "|" was not correctly handled by BSD sed (used e.g. in MacOSX).

So maybe it would be best to just look for "commit message"?  I agree with
Michael that the regex should not be too loose.
Thanks for pointing this out.  I replaced the problematic part of the
regexp with "[snrt][tdh]", which isn't quite so selective but should
be portable.  (This is the same fix that Junio suggested.)

Björn Gustavsson wrote:
On Fri, Dec 4, 2009 at 3:36 PM, Michael Haggerty [off-list ref] wrote:
Nitpick: the recommended style is to leave out the full stop
at the end of the first line of the commit message.
Nit picked.

Junio C Hamano wrote:
IIRC, the end result of the bikeshedding for the name of the command was
won by Dscho's "fixup":

  http://thread.gmane.org/gmane.comp.version-control.git/127923/focus=121820
That's fine with me (the abbreviation is even the same :-) ).
Changed.

Michael

Michael Haggerty (2):
  t3404: Use test_commit to set up test repository
  Add a command "fixup" to rebase --interactive

 Documentation/git-rebase.txt  |   13 +++--
 git-rebase--interactive.sh    |   51 +++++++++++++++++----
 t/lib-rebase.sh               |    7 ++-
 t/t3404-rebase-interactive.sh |   96 +++++++++++++++++++++-------------------
 4 files changed, 103 insertions(+), 64 deletions(-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help