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

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

[Paul Fredrickson]

> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > 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.
>
> Why not use the mark syntax that fast-import uses?  In fast-import
> we use ":n" anytime we need to refer to a mark, e.g. ":1" or ":5".
> Its the same idea.  We already have a language for it.  Heck, the
> commands above are bordering on a language not too far from the
> one that fast-import accepts.  :-)

I like the idea of adding marks to an interactive rebase in general, but instead
of adding a separate command, what if rebase *automatically* marked all the
commits in the session:

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

or "reset :1" and "merge :2".  Neither notation bothers me for marks.

--Paul

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

[Jörg Sommer]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

Hello Paul,

Paul Fredrickson schrieb am Sun 13. Apr, 13:51 (-0700):
> > Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > > Wouldn't
> > > >
> > > > pick 5cc8f37 (init: show "Reinit" message even in ...)
> > > > mark 1
> > > > pick 18d077c (quiltimport: fix misquoting of parse...)
> > > > mark 2
> > > > reset 1
> 
> I like the idea of adding marks to an interactive rebase in general, but instead
> of adding a separate command, what if rebase *automatically* marked all the
> commits in the session:
> 
>     1: pick 5cc8f37 (init: show "Reinit" message even in ...)
>     2: pick 18d007c (quiltimport: fix misquoting of parse ...)
>     reset 1
>     merge 2

This format would be incompatible with the current format and it makes
the parsing a little bit more difficult; the first column contains a mark
or a command. No, I think that's not a good idea.

Have a nice day, Jörg.
-- 
Der Hase läuft schneller als der Fuchs,
denn der Hase läuft um sein Leben.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

[Johannes Schindelin]

Hi,

On Sun, 13 Apr 2008, Paul Fredrickson wrote:

> > Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > > 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?

Actually, I think that this whole "mark" stuff is way too complicated, as 
can be seen by the amount of patches needed to get it somewhere usable.

I would like it much better, if there was something like

pick 5cc8f37 (init: show "Reinit" message even in ...)
pick 18d077c (quiltimport: fix misquoting of parse...)
merge 9876543:5cc8f37,18d077c (Merge blub)
reset 5cc8f37
...

I.e. like with filter-branch, and like with rebase -i -p in its current 
form, we take the _original_ names as keys as to which commits to merge, 
or where to reset to.

That would be relatively easy to implement, since the whole infrastructure 
for it is already there: whenever a commit was rewritten, the new commit 
name is saved in $DOTEST/rewritten/<original-commit-name>.

I really do not like complicating things unnecessarily.

Ciao,
Dscho

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I would like it much better, if there was something like
>
> pick 5cc8f37 (init: show "Reinit" message even in ...)
> pick 18d077c (quiltimport: fix misquoting of parse...)
> merge 9876543:5cc8f37,18d077c (Merge blub)
> reset 5cc8f37
> ...
>
> I.e. like with filter-branch, and like with rebase -i -p in its current 
> form, we take the _original_ names as keys as to which commits to merge, 
> or where to reset to.

While the need probably would not be felt strongly if we design this only
for rebase -i, I suspect that you would want to have two kinds of reset if
you go that route.  There might be some other insn that may have similar
issues.

For example, imagine a case where you want to create a merge with a
recontructed side branch.  First you grow the branch you would merge into,
with a sequence:

	pick A
        pick B
        pick C

Then in order to reconstruct a side branch that begins from a known point,
say the tip of "master", you would want to reset to a commit that is
outside of the scope of this rewriting.  And then you rebuild that side
branch:

	reset master
        pick D
        pick E

And finally (and this step shows the beauty of your approach), come back
to the other tip and make the merge:

	reset C
        merge E

Two resets above would have different semantics.  The former resets to
unwritten, and the latter rewritten.

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> Me too, but I think it's not possible to do what I want with -p. -p
> misses a definition of the (new) parent of a commit. It tries to preserve
> all commits from all branches. But going through the _list_ of commands
> couldn't preserve this structure.
>
> o--A--B
>  \     \
>   C--D--M--E
>
> How should the graph look like after these commands:
>
> pick A
> pick C
> squash E
> # pick D
> pick B
> pick M

I am beginning to suspect that the root cause of this is that the todo
language is not expressive enough to reproduce a merge _and_ allow end
user editing.

Let's step back a bit.

If you have this history:

      o---o---o---o---o---Z
     /
    X---Y---A---B
         \       \
          C---D---M---E

and you want to transplant the history  between X..E on top of Z, from the
command line you would say:

	$ git rebase --interactive -p --onto Z X E

First let's think what you would do if you want to do this by hand.  The
sequence would be:

	$ git checkout Z^0 ;# detach at Z

        $ git cherry-pick Y
        $ git tag new-Y ;# remember it
        $ git cherry-pick A
        $ git cherry-pick B
        $ git tag new-B ;# remember it
        $ git checkout new-Y
        $ git cherry-pick C
        $ git cherry-pick D
        $ git merge new-B ;# this reproduces M
        $ git cherry-pick E

	$ git branch -f $the_branch && git checkout $the_branch


Now how does the todo file before you edit look like?

	pick Y
        pick A
        pick B
        pick C
        pick D
        pick M
        pick E

The todo file expects the initial detaching and the final switching back
outside of its control, so it is Ok that the first "checkout Z^0" and the
last "branch && checkout" do not appear, but it should be able to express
the remainder and let you tweak.  Is it expressive enough to do so?  

Most of the "pick" from the above list literally translate to the
"cherry-pick", and if you change any of them to "edit", that is
"cherry-pick --no-edit" followed by a return of control to you with insn
to "rebase --continue" to resume.  There appears nothing magical.

Not really.  There already are two gotchas even without talking about
end-user editing.

First, "pick M" is not "cherry-pick M".  You do not want to end up with
merging an old parent before rewriting.  It has to be something like
"merge rewritten-Y".

Second, before you start picking C, if you want to preserve merges, you
have to switch to rewritten Y.  The original sequence left in todo does
not have that information to begin with.  We need, before the "pick C", to
say the equivalent of "git checkout new-Y" in the manual sequence
illustrated above.  The lack of "checkout new-Y" is perfectly fine if
rebase is meant to linearlize the history, but if you want to preserve the
shape of the history, you would need to give a clue that the sequence that
begins with the "pick C" starts from somewhere else.

You also need to make sure that "pick M" moved elsewhere still merges the
tips of two forked histories.  Moving "pick M" before "pick C" or "pick A"
would not make much sense.  So you would need some kind of "barrier" that
says "do not move this 'pick M' beyond this point".

Perhaps we can make it clearer by introducing a few more primitives to the
todo language: mark, reset and merge.  The above illustrated history would
then become:

	pick Y
        mark #0
	pick A
        pick B
        mark #1
        reset #0
        pick C
        pick D
	mark #2
        merge #1 #2
        pick E

You can change any of the "pick" to "edit, or drop it, and you can reorder
"pick" in a sequence of "pick", but you cannot change "mark", "reset",
"merge", or move "pick" across insn that was not originally "pick".

Teach rebase interactive more commands to do better preserve merges

[Jörg Sommer]

Junio proposed to add the commands mark, merge and reset to rebase
interactive for a better support of rebase with preserve merges. The
current format can only cope with flat lineare lists of commits and not
with the non‐linear structure of branches with merges.

This patch series is not intended for inclusion. It's a RFC and meant for
gathering ideas.

Currently it misses:
· commit messages
· documentation of the new commands
· update of the documentation of preserve merges
· tests for the merge command

I've included the patch to change the behaviour of git-merge from Junio,
because I don't know how to use the second syntax of git merge and pass
the merge strategies -s to git merge. Without this patch, a new merge
message gets appended to the commit message of a rebased merge commit.
This patch should not be part of the final commit series.

Open questions:
· Is it possible to get a list of all commits iterating the parents from
  the last to the first and going to the child of a node before the
  neighbors?

· How to get a symbolic name (branch or tag) for a commit? Not something
  like git describe. I want to know if 0123 refers to the tip of a
  branch.

Jörg.

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

[Jörg Sommer]

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.
---
 git-rebase--interactive.sh    |   36 ++++++++++++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh |   21 +++++++++++++++++++++
 2 files changed, 57 insertions(+), 0 deletions(-)

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
 PRESERVE_MERGES=
 STRATEGY=
 VERBOSE=
@@ -240,6 +241,23 @@ peek_next_command () {
 	sed -n "1s/ .*$//p" < "$TODO"
 }
 
+parse_mark() {
+	local tmp
+	tmp="$*"
+
+	case "$tmp" in
+	'#'[0-9]*)
+		tmp="${tmp#\#}"
+		if test "$tmp" = $(printf %d "$tmp")
+		then
+			echo $tmp
+			return 0
+		fi
+		;;
+	esac
+	return 1
+}
+
 do_next () {
 	rm -f "$DOTEST"/message "$DOTEST"/author-script \
 		"$DOTEST"/amend || exit
@@ -317,6 +335,23 @@ do_next () {
 			die_with_patch $sha1 ""
 		fi
 		;;
+	mark|a)
+		if ! mark=$(parse_mark $sha1)
+		then
+			warn "Invalid mark given: $command $sha1 $rest"
+			die_with_patch $sha1 \
+				"Please fix this in the file $TODO."
+		fi
+		mark_action_done
+
+		test -d "$MARKS" || mkdir "$MARKS"
+
+		test -e "$MARKS"/$mark && \
+			warn "mark $mark already exist; overwriting it"
+
+		git rev-parse --verify HEAD > "$MARKS"/$mark || \
+			die "HEAD is invalid"
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		die_with_patch $sha1 "Please fix this in the file $TODO."
@@ -533,6 +568,7 @@ do
 #  pick = use commit
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
+#  mark #NUM = mark the current HEAD for later reference
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9cf873f..4461674 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -82,6 +82,9 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|edit)
 		action="$line";;
+	mark*)
+		echo "mark ${line#mark}"
+		echo "mark ${line#mark}" >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
@@ -189,6 +192,24 @@ test_expect_success '-p handles "no changes" gracefully' '
 	test $HEAD = $(git rev-parse HEAD)
 '
 
+test_expect_success 'setting an invalid mark fails' '
+	export FAKE_LINES="mark12 1" && \
+	test_must_fail git rebase -i HEAD~1 &&
+	unset FAKE_LINES &&
+	git rebase --abort
+'
+
+test_expect_success 'setting marks works' '
+	git checkout master &&
+	FAKE_LINES="mark#0 2 1 mark#42 3 edit 4" git rebase -i HEAD~4 &&
+	marks_dir=.git/.dotest-merge/marks
+	test -d $marks_dir &&
+	test $(ls $marks_dir | wc -l) -eq 2 &&
+	test "$(git rev-parse HEAD~4)" = "$(cat $marks_dir/0)" &&
+	test "$(git rev-parse HEAD~2)" = "$(cat $marks_dir/42)" &&
+	git rebase --abort
+'
+
 test_expect_success 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
-- 
1.5.4.5

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

[Mike Ralphson]

On 10/04/2008, Jörg Sommer <joerg@alea.gnuu.de> wrote:
> 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.

What would be wrong with using the existing tag machinery for this instead?

Mike

PS: Apologies to anyone who got an html version of this mail.

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

[Jörg Sommer]

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

Hallo Mike,

Mike Ralphson schrieb am Thu 10. Apr, 10:33 (+0100):
> On 10/04/2008, Jörg Sommer <joerg@alea.gnuu.de> wrote:
> > 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.
> 
> What would be wrong with using the existing tag machinery for this instead?

You may have to deal with conflicts if users named tags 03 or 10. But
Junio suggested to use a ref, too. I think refs/rebase-marks/ is a good
prefix.

Bye, Jörg.
-- 
Der Hase läuft schneller als der Fuchs,
denn der Hase läuft um sein Leben.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> 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...

> 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.

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

Bashism is not appreciated here.

> +	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
        merge #2

be easier for people?

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.

> @@ -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.

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

[Jörg Sommer]

[-- Attachment #1: Type: text/plain, Size: 3580 bytes --]

Hi Junio,

Junio C Hamano schrieb am Fri 11. Apr, 16:48 (-0700):
> Jörg Sommer <joerg@alea.gnuu.de> 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 #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

[Shawn O. Pearce]

Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > 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.

Why not use the mark syntax that fast-import uses?  In fast-import
we use ":n" anytime we need to refer to a mark, e.g. ":1" or ":5".
Its the same idea.  We already have a language for it.  Heck, the
commands above are bordering on a language not too far from the
one that fast-import accepts.  :-)

-- 
Shawn.

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

[Jörg Sommer]

[-- Attachment #1: Type: text/plain, Size: 1291 bytes --]

Hi Shawn,

Shawn O. Pearce schrieb am Sat 12. Apr, 23:56 (-0400):
> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > 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.
> 
> Why not use the mark syntax that fast-import uses?

I didn't know it.

> In fast-import we use ":n" anytime we need to refer to a mark, e.g.
> ":1" or ":5".

Currently, I don't restrict the mark to be a number. It can anything that
is a valid ref. Should I restrict it?

And how do you handle the :/ syntax? “reset :/Bla” is than not valid.
Mmm, I'll add an exception for :/.

Except of this, I prefer to use the colon to be much closer to the syntax
of fast-import.

Bye, Jörg.
-- 
Der Wunsch, klug zu erscheinen, verhindert oft, es zu werden.
    	    	    			      (Francois de la Rochefoucauld)

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

[Shawn O. Pearce]

Jrg Sommer <joerg@alea.gnuu.de> wrote:
> Shawn O. Pearce schrieb am Sat 12. Apr, 23:56 (-0400):
> > 
> > Why not use the mark syntax that fast-import uses?
> 
> I didn't know it.
> 
> > In fast-import we use ":n" anytime we need to refer to a mark, e.g.
> > ":1" or ":5".
> 
> Currently, I don't restrict the mark to be a number. It can anything that
> is a valid ref. Should I restrict it?

In fast-import a mark can *only* be a number.  It cannot be a ref
string or anything complex like that.  This reduces the memory load
of fast-import, but does cause a burden on the import frontend.
 
> And how do you handle the :/ syntax? “reset :/Bla” is than not valid.
> Mmm, I'll add an exception for :/.

I think the ':/' syntax came along after fast-import had already started
to use ':' as the mark syntax.  I forgot to object to this bastard form
of looking up a commit when it was introduced by Dscho and now we have
a SHA-1 expression syntax that fast-import will confuse with a mark.  I
originally had chosen to start a mark off with ':' because it is not an
allowed character in a ref, due to its use to split src:dst in a fetch
or push refspec.
 
> Except of this, I prefer to use the colon to be much closer to the syntax
> of fast-import.

Me too, but it looks like in a human edited "TODO" script we may want
to be more friendly and allow named marks.  Though I'm not sure that is
really all that useful.  If you are merging something because it used to
be merged before the rebase I doubt we'd generate a meaningful mark name
when the TODO script is initially formatted.

-- 
Shawn.

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

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

>> Except of this, I prefer to use the colon to be much closer to the syntax
>> of fast-import.
>
> Me too, but it looks like in a human edited "TODO" script we may want
> to be more friendly and allow named marks.  Though I'm not sure that is
> really all that useful.  If you are merging something because it used to
> be merged before the rebase I doubt we'd generate a meaningful mark name
> when the TODO script is initially formatted.

I'd say a small integer is the only thing we would need.  The TODO insn
sequence would be machine generated then manually tweaked, not the other
way around.

Regarding colon vs pound, I would say that the 'mark' insn can just say
"2" to set the second mark, and then store it in refs/marks/2; the
commands that refer to the commit later can use the usual "refs/marks/2"
or "marks/2" syntax without colon nor pound nor any other special syntax
that way.

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

[Jörg Sommer]

[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]

Hi Shawn,

Shawn O. Pearce schrieb am Mon 14. Apr, 02:24 (-0400):
> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > Shawn O. Pearce schrieb am Sat 12. Apr, 23:56 (-0400):
> > > 
> > > Why not use the mark syntax that fast-import uses?
> > 
> > I didn't know it.
> > 
> > > In fast-import we use ":n" anytime we need to refer to a mark, e.g.
> > > ":1" or ":5".
> > 
> > Currently, I don't restrict the mark to be a number. It can anything that
> > is a valid ref. Should I restrict it?
> 
> In fast-import a mark can *only* be a number.

Only for the record: fast-import uses numbers and not strings of
digits, i.e. 001 == 1, and it ignores stuff following digits, i.e.
"12a" == 12 and "abc" == 0.

> > Except of this, I prefer to use the colon to be much closer to the syntax
> > of fast-import.
> 
> Me too, but it looks like in a human edited "TODO" script we may want
> to be more friendly and allow named marks.

I don't think so. There shouldn't be so much marks that a user can't
remember them.

Thanks for your comments.

Jörg.
-- 
The UNIX Guru's View of Sex:
# unzip ; strip ; touch ; finger ; mount ; fsck ; more ; yes ; umount ; sleep

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]