Hi Junio,

Junio C Hamano schrieb am Fri 11. Apr, 16:48 (-0700):
quoted text
> J=C3=B6rg Sommer <joerg@alea.gnuu.de> writes:
>=20
> > 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.
> > ---
>=20
> Comments as requested...

Thanks.

quoted text
> > 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=3D"$DOTEST"/done
> >  MSG=3D"$DOTEST"/message
> >  SQUASH_MSG=3D"$DOTEST"/message-squash
> >  REWRITTEN=3D"$DOTEST"/rewritten
> > +MARKS=3D"$DOTEST"/marks
>=20
> 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 text
> > @@ -240,6 +241,23 @@ peek_next_command () {
> >  	sed -n "1s/ .*$//p" < "$TODO"
> >  }
> > =20
> > +parse_mark() {
> > +	local tmp
>=20
> Bashism is not appreciated here.

=E2=80=9CIEEE P1003.2 Draft 11.2 - September 1991=E2=80=9D, 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.

=E2=80=A6 but I didn't found it in =E2=80=9CIEEE Std 1003.1, 2004 Edition=
=E2=80=9D.

quoted text
> > +	case "$tmp" in
> > +	'#'[0-9]*)
> > +		tmp=3D"${tmp#\#}"
> > +		if test "$tmp" =3D $(printf %d "$tmp")
> > +		then
> > +			echo $tmp
> > +			return 0
> > +		fi
> > +		;;
> > +	esac
>=20
> Wouldn't
>=20
> 	pick 5cc8f37 (init: show "Reinit" message even in ...)
> 	mark 1
> 	pick 18d077c (quiltimport: fix misquoting of parse...)
> 	mark 2
> 	reset 1

=E2=80=9Creset 18d077c~2=E2=80=9D or =E2=80=9Creset some-tag=E2=80=9D or =
=E2=80=9Creset my-branch~12=E2=80=9D

quoted text
>         merge #2
>=20
> 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.

quoted text
> 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 =3D=3D 1 or 001 !=3D=
 1?

quoted text
> > @@ -317,6 +335,23 @@ do_next () {
> >  			die_with_patch $sha1 ""
> >  		fi
> >  		;;
> > +	mark|a)
>=20
> 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=C3=B6rg.
--=20
Was der Bauer nicht kennt, das frisst er nicht. W=C3=BCrde der St=C3=A4dter=
 kennen,
was er frisst, er w=C3=BCrde umgehend Bauer werden.
                                                       Oliver Hassencamp