Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

Hi Junio,

Junio C Hamano schrieb am Fri 11. Apr, 16:48 (-0700):

Jörg Sommer [off-list ref] writes:

quoted

This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.
---

Comments as requested...

Thanks.

quoted

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8aa7371..b001ddf 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh

@@ -23,6 +23,7 @@ DONE="$DOTEST"/done
 MSG="$DOTEST"/message
 SQUASH_MSG="$DOTEST"/message-squash
 REWRITTEN="$DOTEST"/rewritten
+MARKS="$DOTEST"/marks

I wonder if this should live somewhere inside $GIT_DIR/refs namespace, so
that if any object pruning is triggered ever while rebasing interactively
the objects that marks require will be protected.

Why wasn't the rewritten directory underneath refs/?

quoted

@@ -240,6 +241,23 @@ peek_next_command () {
 	sed -n "1s/ .*$//p" < "$TODO"
 }
 
+parse_mark() {
+	local tmp

Bashism is not appreciated here.

“IEEE P1003.2 Draft 11.2 - September 1991”, page 277:

 Local variables within a function were considered and included in Draft
 9 (controlled by the special built-in local), but were removed because
 they do not fit the simple model developed for the scoping of functions
 and there was some opposition to adding yet another new special built-in
 from outside existing practice.  Implementations should reserve the
 identifier local (as well as typeset, as used in the KornShell) in case
 this local variable mechanism is adopted in a future version of POSIX.2.

… but I didn't found it in “IEEE Std 1003.1, 2004 Edition”.

quoted

+	case "$tmp" in
+	'#'[0-9]*)
+		tmp="${tmp#\#}"
+		if test "$tmp" = $(printf %d "$tmp")
+		then
+			echo $tmp
+			return 0
+		fi
+		;;
+	esac

Wouldn't

	pick 5cc8f37 (init: show "Reinit" message even in ...)
	mark 1
	pick 18d077c (quiltimport: fix misquoting of parse...)
	mark 2
	reset 1

“reset 18d077c~2” or “reset some-tag” or “reset my-branch~12”

        merge #2

be easier for people?

I don't know. Using the special sign everywhere a mark is used looks more
consistent to me. The only case where it might be omitted is the mark
command, because it only uses marks.

When you reference a commit with mark name, it is reasonable to require it
to be prefixed with '#', which is a character that cannot be either in
refname nor hex representation of a commit object.  I think it is Ok to
accept an optional prefix when defining one, e.g. "mark #47", but I do not
think requiring '#' prefix when defining one is needed.

That sounds useful.

BTW: Should the mark be a number or a string, e.g. 001 == 1 or 001 != 1?

quoted

@@ -317,6 +335,23 @@ do_next () {
 			die_with_patch $sha1 ""
 		fi
 		;;
+	mark|a)

I understand that the reason why you did not pick a more obvious 'm' is
because you would want to add 'merge' later and give it 'm', but we do not
have to give a single-letter equivalent to all commands especially when
there is not an appropriate one.

That's fine. I thought it's a requirement.

Bye, Jörg.
-- 
Was der Bauer nicht kennt, das frisst er nicht. Würde der Städter kennen,
was er frisst, er würde umgehend Bauer werden.
                                                       Oliver Hassencamp