Hi Junio,
Junio C Hamano schrieb am Fri 11. Apr, 16:48 (-0700):
> Jörg Sommer <joerg@xxxxxxxxxxxx> writes:
>
> > 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.
> > 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/?
> > @@ -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”.
> > + 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?
> > @@ -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
Attachment:
signature.asc
Description: Digital signature http://en.wikipedia.org/wiki/OpenPGP
