Re: [PATCH 2/2] Fix t3404 assumption that `wc -l` does not use whitespace.

["Jörg Sommer" <joerg@alea.gnuu.de>]

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

Hi,

Johannes Schindelin schrieb am Mon 28. Apr, 14:42 (+0100):
> On Mon, 28 Apr 2008, Jörg Sommer wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > On Sun, 27 Apr 2008, Junio C Hamano wrote:
> > >
> > >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > >> 
> > >> > ...  It did not help that I hated the fact that that series changed 
> > >> > the original design without even understanding it.
> > >> 
> > >> Care to elaborate on this point further?  I do not get it.
> > >
> > > The original implementation of -p was modeled closely after 
> > > filter-branch, in that it created a subdirectory (dotest/rewritten) 
> > > containing the new commit names for those commits that were rewritten.
> > 
> > But that wasn't the way rebase -i works.
> 
> I know exactly how it works. D'oh.
> 
> > You had to jump in before pick_one does anything which clearly shows you 
> > did something different from the default way.
> 
> That is bullshit.  I did not do anything "different from the default way".  
> I carefully designed an interface that was easy to understand, because it 
> mimicked how you would do the same _by hand_, but without the hassle to 
> actually having to do everything by hand.
> 
> In other words, rebase -i is just a cherry-pick in a loop.

But not rebase -i -p.

> And _exactly_ the same should have been done for -p.

But you didn't do it.

> Namely, _not_ introduce some artificial marks, but use the _commit
> names_!

I don't buy, you don't use marks (notes on paper or git tags) when you rebase
a branch with at least 8 commits and 2 merges.

And Junio discribed how he would do such a rebase and it included marks.
And I follow how. So no, they aren't artificial.

> > > Now, whenever a commit was picked, the parents would be looked up in 
> > > dotest/rewritten, and replaced with the rewritten name (or left 
> > > unchanged if they were not rewritten).
> > 
> > This approach doesn't work when you change the order of commits.
> > Take the commit A, B and C in this order and reorder them to A C B:
> > 1. pick A, A^ was not rewritten, nothing changed, A stays the same
> > 2. pick C, C^ was not rewritten, nothing changed, C stays the same
> > 3. pick B, B^ was not rewritten, nothing changed, B stays the same
> 
> You carefully ignored how I intended the parents to be used: only for 
> merges.

And why does this test fail? Please tell me, as you “know exactly how it
works.”

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9cf873f..83c2964 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -361,4 +361,10 @@ test_expect_success 'rebase with a file named HEAD in worktree' '
 
 '
 
+test_expect_success 'rebase with a file named HEAD in worktree' '
+       head=$(git rev-parse HEAD) &&
+       FAKE_LINES="1 3 2" git rebase -i -p HEAD~3 &&
+       test $(git rev-parse HEAD) != $head
+'
+
 test_done

* FAIL 25: rebase with a file named HEAD in worktree

                head=$(git rev-parse HEAD) &&
                FAKE_LINES="1 3 2" git rebase -i -p HEAD~3 &&
                test $(git rev-parse HEAD) != $head

> > > Basically, the output of rebase -i -p is ugly now, because you have 
> > > _two_ ways of specifying things,
> > 
> > > I have the feeling that I have to repeat my point again, so that it is not 
> > > ignored -- again.  Maybe an example would help:
> > >
> > > -- snip --
> > > pick abcdefg This is the first commit to be picked
> > > reset cdefghij
> > > pick zyxwvux A commit in a side-branch
> > > merge recursive abcdefg

Where do you tell which merge should be redone?

> > > -- snap --
> > >
> > > I am convinced that this syntax does not need much explanation.
> > 
> > But above you said this syntax + mark is “ugly”. Strange.
> 
> You know, I find it strange how you try to make a _point_ in 
> misunderstanding me.  Did I not mention that the way to have _two_ ways to 
> reference commits was ugly?  You did not even bother to remove that part 
> from what you quoted.

Because Junio told you about it. And I don't find your suggestion in
<alpine.DEB.1.00.0804141506270.28504@racer> very readable:

| 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 don't see why you complain about the marks and suggest to use
9876543:5cc8f37,18d077c. In a short example like yours it doesn't hurd,
but put 10 line between the merge an the pick and maybe change move one
of the merged commits behind the merge.

pick 8a785dc Add tests to catch problems with un-unlinkable symlinks
pick 8d14ac9 Test: catch if trash cannot be removed
pick 29dc133 git-merge-one-file: fix longstanding stupid thinko
pick deda26b Merge branch 'jc/makefile'
pick 7f8ab8d Don't update unchanged merge entries
pick 198724a fast-import: Allow "reset" to delete a new branch without error
pick 20fd60b t1000: use "test_must_fail git frotz", not "! git frotz"
pick 7092882 Update draft release notes for 1.5.5
pick c817faa Resurrect git-rerere to contrib/examples
pick 1eaa541 Merge branch 'maint'
pick 81d6650 Start draft ReleaseNotes for 1.5.4.5
merge 9876543:81d6650,198724a (Merge blub)
pick e637122 rebase -m: do not trigger pre-commit verification

pick 8a785dc Add tests to catch problems with un-unlinkable symlinks
pick 8d14ac9 Test: catch if trash cannot be removed
pick 29dc133 git-merge-one-file: fix longstanding stupid thinko
pick deda26b Merge branch 'jc/makefile'
pick 7f8ab8d Don't update unchanged merge entries
pick 198724a fast-import: Allow "reset" to delete a new branch without error
mark #1
pick 20fd60b t1000: use "test_must_fail git frotz", not "! git frotz"
pick 7092882 Update draft release notes for 1.5.5
pick c817faa Resurrect git-rerere to contrib/examples
pick 1eaa541 Merge branch 'maint'
pick 81d6650 Start draft ReleaseNotes for 1.5.4.5
merge 9876543:81d6650,#1 (Merge blub)
pick e637122 rebase -m: do not trigger pre-commit verification

> > > A patch implementing a syntax like this would have won my unilateral 
> > > approval
> > 
> > I doubt this. You refused any changes to your idea and your code from 
> > the beginning. You didn't answer questions and doesn't take part on the 
> > discussion [1] about the new syntax.
> 
> Well, you carefully ignored (but removed from the quoted text) my 
> explanation.

Sorry no, Junio made his proposal on Mar 24 and merge the code on Apr 25.
I treat this as a adequate time window to propose a _better_ idea.

Bye, Jörg.
-- 
“Unfortunately, the current generation of mail programs do not have
 checkers to see if the sender knows what he is talking about”
            (Andrew S. Tanenbaum)

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