Git development
 help / color / mirror / Atom feed
* Re: rebase -p confusion in 1.6.1
From: Johannes Schindelin @ 2009-01-15 16:04 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Stephan Beyer, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <496F4BF0.6020805@drmicha.warpmail.net>

Hi,

On Thu, 15 Jan 2009, Michael J Gruber wrote:

> I'm not sure what -p is supposed to do:
> 
> A) Should it preserve all merge commits which it would need to rewrite?
> That is lot to ask. Previous behaviour (intended or not) seemed to be to
> do nothing in this case where the merge connects master and work.
> 
> B) Should it preserve only merges in side branches? I seem to mean by
> that branches where the parents are on work and other branches but not
> on master.

The intention was this:

	$ git rebase -p master

would need to rewrite _all_ commits that are in "master..".  All of them, 
including the merge commits.

The fact that I implemented it as "-i -p" is only due to technical 
reasons; I know (ahem, now I should put that into the past tense) the code 
base pretty well.

An additional shortcut was to avoid rewriting commits when they did not 
need rewriting.  IOW if the commit-to-pick has only parents that were 
_not_ rewritten, we can avoid cherry-picking or merging, and just reset 
--hard <original commit>.

There was a problem, though; for some reason, the code as I did it fscked 
up the order of the commits when -p was specified.  Therefore, rewritten 
commits had the wrong parents.

I thought it should be easy to fix, but then I got a job that I actually 
like, so my Git time budget was tremendously reduced.

> > The more I think about it, I think it's possible I broke it with the 
> > introduction of the "noop".
> 
> It certainly worked after the noop introduction before the r-i-p series, 
> but not any more after.

Umm... which rebase -i -p series do you mean?  "noop" was introduced 
pretty recently if my Alzheimered brain does not fool me.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 2/2] api-run-command.txt: talk about run_hook()
From: Miklos Vajna @ 2009-01-15 16:12 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Stephan Beyer, git, Paolo Bonzini, Shawn O. Pearce,
	Daniel Barkalow, Christian Couder, gitster
In-Reply-To: <m38wpczi09.fsf@localhost.localdomain>

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

On Thu, Jan 15, 2009 at 07:49:51AM -0800, Jakub Narebski <jnareb@gmail.com> wrote:
> > +	The further variable number (up to 9) of arguments correspond
> > +	to the hook arguments.
> > +	The last argument has to be NULL to terminate the variable
> > +	arguments list.
> 
> Why the limitation of maximum of 9 hook arguments?

That's how it is in builtin-commit at the moment, and I agree with Dscho
about it could be a separate patch to remove this limitation.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Sitaram Chamarty @ 2009-01-15 16:24 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.DEB.1.00.0901151658060.3586@pacific.mpi-cbg.de>

On 2009-01-15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> The intention was this:
>
> 	$ git rebase -p master
>
> would need to rewrite _all_ commits that are in "master..".  All of them, 
> including the merge commits.

I went hog wild with all sorts of test cases and my head is
spinning, but -- even when things happen more predictably,
I'm unable to make "rebase -p" carry an evil merge over.
The "evil" part stays behind.

I'm not sure if that is intentional or not, or (more likely)
my brain has become addled and I missed something somewhere.

Regards,

Sitaram

^ permalink raw reply

* Re: [PATCH v2] checkout: implement "-" shortcut name for last branch
From: Johan Herland @ 2009-01-15 16:32 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Johannes Sixt, Thomas Rast, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0901151413250.3586@pacific.mpi-cbg.de>

On Thursday 15 January 2009, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 15 Jan 2009, Johannes Sixt wrote:
> > Thomas Rast schrieb:
> > > Let git-checkout save the old branch as a symref in LAST_HEAD,
> > > and make 'git checkout -' switch back to LAST_HEAD, like 'cd -'
> > > does in the shell.
> >
> > /me likes this feature.
> >
> > git rebase (-i or not) calls checkout behind the scenes if the
> > two-argument form is used:
> >
> >    git rebase [-i] master topic
> >
> > and 'topic' is not the current branch. You may want to add a test
> > that ensures that rebase sets LAST_HEAD in this case.
> >
> > You must make sure that commits referenced by LAST_HEAD are not
> > garbage-collected. (I don't know if this happens anyway for symrefs
> > in .git.)
>
> Note: if you used reflogs for that feature, the garbage collection
> could not have killed the commit.  However, it is quite possible that
> the branch was deleted.

I also like this feature, but as this is only a _convenience_ feature, I 
would prefer if it didn't keep the previous branch/commit alive (if 
otherwise unreachable). In any case, this new feature will _have_ to 
handle the case where there simply is no previous branch/commit (e.g. 
after a git clone or git init).

I suggest that "git checkout -" looks at the reflog, and if there is no 
previous entry in the reflog, or that entry is unreachable, then fail 
in the same manner as "git checkout garbage"


Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Johannes Schindelin @ 2009-01-15 16:43 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Michael J Gruber, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <20090115144050.GD10045@leksak.fem-net>

Hi,

On Thu, 15 Jan 2009, Stephan Beyer wrote:

> Michael J Gruber wrote:
> > If it's about to be integrated we can do without the
> > present script...
> 
> I think it will take some time and some discussions on the list until it 
> will be integrated.  I remember, for example, Dscho, who has, since it 
> had first come up, always been opposed to the mark-reset / 
> mark-reset-merge scheme (in rebase -i -p, at least). Other users said 
> "Wow, this is much more flexible." ... and this is perhaps only one 
> thing that can lead to some bigger discussion.

Wow, much more flexible.  Except that you should not need this kind of 
flexibility.  If you need to do something complicated, it would be better 
to use "rebase -i -p" for the parts that do _not_ need to pick _other_ 
parents than are recorded in the commits.

And then you do an "edit" (or "pause" or whatever), and cherry-pick/merge 
_explicitely_ what you want.

Further, keep in mind that not only is that flexibility of dubitable value 
to the most users, it is also confusing, _and_ it adds code that is so 
rarely exercized that bugs can lurk in there for years... as you can 
experience right now.

So no, nothing has changed, I find that mark idea still horrible, 
horrible, horrible.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v2] checkout: implement "-" shortcut name for last branch
From: Johannes Schindelin @ 2009-01-15 16:50 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, Johannes Sixt, Thomas Rast, Junio C Hamano
In-Reply-To: <200901151732.57023.johan@herland.net>

Hi,

On Thu, 15 Jan 2009, Johan Herland wrote:

> On Thursday 15 January 2009, Johannes Schindelin wrote:
>
> > On Thu, 15 Jan 2009, Johannes Sixt wrote:
> > > Thomas Rast schrieb:
> > > > Let git-checkout save the old branch as a symref in LAST_HEAD,
> > > > and make 'git checkout -' switch back to LAST_HEAD, like 'cd -'
> > > > does in the shell.
> > >
> > > /me likes this feature.
> > >
> > > git rebase (-i or not) calls checkout behind the scenes if the
> > > two-argument form is used:
> > >
> > >    git rebase [-i] master topic
> > >
> > > and 'topic' is not the current branch. You may want to add a test
> > > that ensures that rebase sets LAST_HEAD in this case.
> > >
> > > You must make sure that commits referenced by LAST_HEAD are not
> > > garbage-collected. (I don't know if this happens anyway for symrefs
> > > in .git.)
> >
> > Note: if you used reflogs for that feature, the garbage collection
> > could not have killed the commit.  However, it is quite possible that
> > the branch was deleted.
> 
> I also like this feature, but as this is only a _convenience_ feature, I 
> would prefer if it didn't keep the previous branch/commit alive (if 
> otherwise unreachable).

You misread me: if the information is in HEAD's reflog, the _is_ 
reachable.  From HEAD's reflog.  And therefore, the objects will not be 
gc'ed (yet).

> In any case, this new feature will _have_ to handle the case where there 
> simply is no previous branch/commit (e.g. after a git clone or git 
> init).
>
> I suggest that "git checkout -" looks at the reflog, and if there is no 
> previous entry in the reflog, or that entry is unreachable, then fail 
> in the same manner as "git checkout garbage"

Exactly my thinking.

Ciao,
Dscho

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Johannes Schindelin @ 2009-01-15 16:53 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: git
In-Reply-To: <slrngmuoq8.3u2.sitaramc@sitaramc.homelinux.net>

Hi,



if you would like me to respond to your questions in the future, it is 
mandatory to keep me in the Cc: list.



On Thu, 15 Jan 2009, Sitaram Chamarty wrote:

> On 2009-01-15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > The intention was this:
> >
> > 	$ git rebase -p master
> >
> > would need to rewrite _all_ commits that are in "master..".  All of them, 
> > including the merge commits.
> 
> I went hog wild with all sorts of test cases and my head is
> spinning, but -- even when things happen more predictably,
> I'm unable to make "rebase -p" carry an evil merge over.
> The "evil" part stays behind.
> 
> I'm not sure if that is intentional or not, or (more likely)
> my brain has become addled and I missed something somewhere.

Yes, this is intentional.

	Instead of ignoring merges, try to recreate them.

That means it tries to recreate them.  Not that it is successful.  And not 
even that it realizes when it failed.

Hth,
Dscho

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Michael J Gruber @ 2009-01-15 16:56 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Stephan Beyer, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <alpine.DEB.1.00.0901151658060.3586@pacific.mpi-cbg.de>

Johannes Schindelin venit, vidit, dixit 15.01.2009 17:04:
...
>>> The more I think about it, I think it's possible I broke it with the 
>>> introduction of the "noop".
>> It certainly worked after the noop introduction before the r-i-p series, 
>> but not any more after.
> 
> Umm... which rebase -i -p series do you mean?  "noop" was introduced 
> pretty recently if my Alzheimered brain does not fool me.

This one introduced noop:

commit ff74126c03a8dfd04e7533573a5c420f2a7112ac
Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date:   Fri Oct 10 13:42:12 2008 +0200

    rebase -i: do not fail when there is no commit to cherry-pick

This is the bad one from bisect:

commit d80d6bc146232d81f1bb4bc58e5d89263fd228d4
Author: Stephen Haberman <stephen@exigencecorp.com>
Date:   Wed Oct 15 02:44:39 2008 -0500

    rebase-i-p: do not include non-first-parent commits touching UPSTREAM

It's the last one in a longer series. And that series is after the noop
introduction.

Michael

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Junio C Hamano @ 2009-01-15 16:59 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Johannes Schindelin, Johan Herland, git, Johannes Sixt,
	Anders Melchiorsen, gitster
In-Reply-To: <bd6139dc0901150445l51f3b861n5bbd85bb6d1382b6@mail.gmail.com>

"Sverre Rabbelier" <srabbelier@gmail.com> writes:

> On Thu, Jan 15, 2009 at 13:36, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> If at all, I'd introduce 'examine' as a synonym to 'edit' (might be more
>> intuitive).
>
> Examine suggests that you cannot change the commit (you can look, but
> don't touch it!), no?

'stop' would be closest to what it currently does.  It stops and it is up
to you how to screw up the result ;-).

^ permalink raw reply

* [JGIT PATCH] Fix RawParseUtils to match on the end of the buffer
From: Shawn O. Pearce @ 2009-01-15 17:02 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git

When we were matching exactly on the end of the buffer the match
method was failing because it incorrectly tested for >= on the
buffer length.  We only needed to test for > the buffer length.

JUnit tests for the match function are included to test for this
condition, and others.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/jgit/util/RawParseUtils_MatchTest.java |   69 ++++++++++++++++++++
 .../src/org/spearce/jgit/util/RawParseUtils.java   |    4 +-
 2 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_MatchTest.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_MatchTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_MatchTest.java
new file mode 100644
index 0000000..32270b7
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/RawParseUtils_MatchTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2009, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.util;
+
+import junit.framework.TestCase;
+
+import org.spearce.jgit.lib.Constants;
+
+public class RawParseUtils_MatchTest extends TestCase {
+	public void testMatch_Equal() {
+		final byte[] src = Constants.encodeASCII(" differ\n");
+		final byte[] dst = Constants.encodeASCII("foo differ\n");
+		assertTrue(RawParseUtils.match(dst, 3, src) == 3 + src.length);
+	}
+
+	public void testMatch_NotEqual() {
+		final byte[] src = Constants.encodeASCII(" differ\n");
+		final byte[] dst = Constants.encodeASCII("a differ\n");
+		assertTrue(RawParseUtils.match(dst, 2, src) < 0);
+	}
+
+	public void testMatch_Prefix() {
+		final byte[] src = Constants.encodeASCII("author ");
+		final byte[] dst = Constants.encodeASCII("author A. U. Thor");
+		assertTrue(RawParseUtils.match(dst, 0, src) == src.length);
+		assertTrue(RawParseUtils.match(dst, 1, src) < 0);
+	}
+
+	public void testMatch_TooSmall() {
+		final byte[] src = Constants.encodeASCII("author ");
+		final byte[] dst = Constants.encodeASCII("author autho");
+		assertTrue(RawParseUtils.match(dst, src.length + 1, src) < 0);
+	}
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index a6a50bb..758e7af 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -71,10 +71,10 @@
 	 *            first position within b, this should match src[0].
 	 * @param src
 	 *            the buffer to test for equality with b.
-	 * @return ptr += src.length if b[ptr..src.length] == src; else -1.
+	 * @return ptr + src.length if b[ptr..src.length] == src; else -1.
 	 */
 	public static final int match(final byte[] b, int ptr, final byte[] src) {
-		if (ptr + src.length >= b.length)
+		if (ptr + src.length > b.length)
 			return -1;
 		for (int i = 0; i < src.length; i++, ptr++)
 			if (b[ptr] != src[i])
-- 
1.6.1.233.g5b4a8

^ permalink raw reply related

* Re: [PATCH] checkout: implement "-" shortcut name for last branch
From: Thomas Rast @ 2009-01-15 17:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0901151510340.3586@pacific.mpi-cbg.de>

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

Johannes Schindelin wrote:
> There are a number of issues why I would like to avoid introducing 
> LAST_HEAD:
> 
> - it does not work when you are using different Git versions on the same 
>   repository,
> 
> - it does not work when you switched recently,

If you switch once, you'll be able to use the feature one checkout
later than if it was reflog-based.

If you switch a lot, the feature won't be in your git half the time
anyway.

> - you are storing redundant information,

AFAIK it's the first instance of this data in a non-free-form field.
There's also the precedent of ORIG_HEAD.

> - yes, the field is meant for user consumption, but no, it is not 
>   free-form,

It's a field of almost arbitrary character data, filled by 70% of the
update-ref calls I can find in git.git in a "<tool>: <comment>" format
and by the rest with things such as "initial pull" or
"refs/remotes/git-svn: updating HEAD".  (The latter is so informative
that it probably deserves a fix.)  How is that not free-form?

> - AFAICT your version could never be convinced to resurrect deleted 
>   branches, without resorting to reflogs anyway.

Neither can any other use of git-checkout without the user manually
recovering some valid revspec referring to the old branch tip from the
reflog.  I wanted to be able to abbreviate the previous branch's name,
and it does just that.

> - the reflog method reflects pretty much exactly how people work around 
>   the lack of "checkout -" currently, so why not just use the same proven 
>   approach?

So you can make me fight an uphill battle against your idea how it
should be done.


-- 
Thomas Rast
trast@{inf,student}.ethz.ch









[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Sverre Rabbelier @ 2009-01-15 17:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Johan Herland, git, Johannes Sixt,
	Anders Melchiorsen
In-Reply-To: <7vmydsv72u.fsf@gitster.siamese.dyndns.org>

On Thu, Jan 15, 2009 at 17:59, Junio C Hamano <gitster@pobox.com> wrote:
> 'stop' would be closest to what it currently does.  It stops and it is up
> to you how to screw up the result ;-).

Hmmm, yes, I think that would be a better alias; but I think I like
the idea to wait for changes like this till sequencer goes in, mhh?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: current git kernel has strange problems during bisect
From: Andreas Bombe @ 2009-01-15 16:54 UTC (permalink / raw)
  To: Kyle Moffett
  Cc: Christian Borntraeger, Linus Torvalds, Sam Ravnborg,
	Johannes Schindelin, git, Linux Kernel Mailing List
In-Reply-To: <f73f7ab80901131226s6af7730cucf9c44bc2b4f9545@mail.gmail.com>

On Tue, Jan 13, 2009 at 03:26:09PM -0500, Kyle Moffett wrote:
> On Sun, Jan 11, 2009 at 4:39 PM, Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
> > In my opinion we should really avoid subtree merges in the future as a curtesy
> > to people who do the uncool work of testing, problem tracking and bisecting.
> > </rant>
> 
> As an alternative, you can relatively easily rewrite the following
> independent histories:
> 
> A -- B -- C
> X -- Y -- Z
> 
> To look like this:
> 
> A -- B -- C -- X' -- Y' -- Z'
> 
> Where X' is (C + sub/dir/X), Y' is (C + sub/dir/Y), etc...

Given that the subtree may have been in development for a long time, it
is almost a certainty that the older commits may compile on A but not
on C.  By basing it all on C you create a lot of uncompilable commits
which hurt bisection just as bad.  At least with missing kernel sources
it is obvious that an attempt at compilation is futile and a waste of
time.

^ permalink raw reply

* Re: [RFC/PATCH 3/7] replace_object: add mechanism to replace objects found in "refs/replace/"
From: Christian Couder @ 2009-01-15 17:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <200901151044.45967.chriscool@tuxfamily.org>

Le jeudi 15 janvier 2009, Christian Couder a écrit :
> Le mardi 13 janvier 2009, Junio C Hamano a écrit :
>
> > Since your paradigm is prepare replacement once at the beginning, never
> > allowing to update it, I think you can update the table while you look
> > it up.  E.g. if A->B and B->C exists, and A is asked for, you find A->B
> > (to tentatively make cur to point at B) and then you find B->C, and
> > before returning you can rewrite the first mapping to A->C.  Later
> > look-up won't need to dereference the table twice that way.
> >
> > This assumes that there will be small number of replacements, but the
> > same object can be asked for more than once during the process.
>
> If we allow different sets of replace refs, for example A->B
> in "refs/replace/" and B->C in "refs/replace/bisect/", then we cannot
> rewrite as you suggest. We could add A->C in "refs/replace/bisect/", so
> that it overcomes A->B and B->C when we bisect, but we would not gain
> much.

Sorry, I just realized I misunderstood what you said. I don't know why but I 
thought you talked about updating the refs. But in fact you are right it 
should be possible to update the "replace_object" table.

Regards,
Christian.

^ permalink raw reply

* Re: [PATCH take 3 0/4] color-words improvements
From: Junio C Hamano @ 2009-01-15 18:15 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Teemu Likonen, Thomas Rast, git, Santi Béjar
In-Reply-To: <alpine.DEB.1.00.0901151337080.3586@pacific.mpi-cbg.de>

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

> BTW this gets even worse when you compare the following:
>
> bbb aaa
> ccc aaa
>
> --color-words=a+ will show
>
> ccc aaa

Naive question.  What is the expected output?

The user defines that "a", "aa", "aaa",... are words and everything else
is the background that the words float on, and asks --color-words to color
code where the words differ.  The way to show them is to have the words in
red (if it comes from preimage) or in green (if it comes from postimage) on
top of some background.

In this case, there is no difference in words, and the only difference is
the background.  Should we still see any output?  Shouldn't it behave more
like "diff -w" that suppresses lines that differ only in whitespace?

I didn't see the semantics of color-words documented in the original
either, and I think it should be described in a way humans would
understand (in other words, "here is what we do internally, splitting
words into lines, running diff between them and coalescing the result in
this and that way, and whatever happens to be output is what you get" is
not the semantics that is explained in a way humans would understand).

The above "The way to show them is to have the words in red (if it comes
from preimage) or in green (if it comes from postimage) on top of some
background." was my attempt to describe an easier half of the semantics,
but I am not sure what definition of "some background" the current draft
code is designed around; I think the original's definition was "we discard
the background from either preimage or postimage and insert whitespace
outselves between the words we output; the only exception is the
end-of-line that appears in the postimage which we try to keep" or
something like that, but that is not written in the documentation either.

How should the background computed to draw the result on?  If a
corresponding background portion appear in both the preimage and the
postimage, we use the one from the postimage?  That justifies why bbb is
not shown but ccc is, when you compare these two:

  bbb aaa
  ccc aa

What happens if a portion of background is only in the preimage?
E.g. when these two are compared:

  bbb aaa bb aa b
  ccc aaa cc

what should happen?  We would want to say "aa" was removed by showing it
in red, but on what background should it be displayed?  cc <red>aa</red>
b?

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Johannes Schindelin @ 2009-01-15 18:18 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Stephan Beyer, Sitaram Chamarty, git, Stephen Haberman
In-Reply-To: <496F6AC3.7050704@drmicha.warpmail.net>

Hi,

On Thu, 15 Jan 2009, Michael J Gruber wrote:

> Johannes Schindelin venit, vidit, dixit 15.01.2009 17:04:
> ...
> >>> The more I think about it, I think it's possible I broke it with the 
> >>> introduction of the "noop".
> >> It certainly worked after the noop introduction before the r-i-p series, 
> >> but not any more after.
> > 
> > Umm... which rebase -i -p series do you mean?  "noop" was introduced 
> > pretty recently if my Alzheimered brain does not fool me.
> 
> This one introduced noop:
> 
> commit ff74126c03a8dfd04e7533573a5c420f2a7112ac
> Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Date:   Fri Oct 10 13:42:12 2008 +0200
> 
>     rebase -i: do not fail when there is no commit to cherry-pick
> 
> This is the bad one from bisect:
> 
> commit d80d6bc146232d81f1bb4bc58e5d89263fd228d4
> Author: Stephen Haberman <stephen@exigencecorp.com>
> Date:   Wed Oct 15 02:44:39 2008 -0500
> 
>     rebase-i-p: do not include non-first-parent commits touching UPSTREAM
> 
> It's the last one in a longer series. And that series is after the noop
> introduction.

Ohhh....

Thanks,
Dscho

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Johannes Schindelin @ 2009-01-15 18:21 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Sverre Rabbelier, Johan Herland, git, Johannes Sixt,
	Anders Melchiorsen
In-Reply-To: <7vmydsv72u.fsf@gitster.siamese.dyndns.org>

Hi,

On Thu, 15 Jan 2009, Junio C Hamano wrote:

> "Sverre Rabbelier" <srabbelier@gmail.com> writes:
> 
> > On Thu, Jan 15, 2009 at 13:36, Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> >> If at all, I'd introduce 'examine' as a synonym to 'edit' (might be more
> >> intuitive).
> >
> > Examine suggests that you cannot change the commit (you can look, but
> > don't touch it!), no?
> 
> 'stop' would be closest to what it currently does.  It stops and it is up
> to you how to screw up the result ;-).

But it shares the first letter with 'squash'.

Ciao,
Dscho

^ permalink raw reply

* Re: rebase -p confusion in 1.6.1
From: Sitaram Chamarty @ 2009-01-15 18:22 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.DEB.1.00.0901151751580.3586@pacific.mpi-cbg.de>

On 2009-01-15, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> if you would like me to respond to your questions in the future, it is 
> mandatory to keep me in the Cc: list.

OK.  [Is that the list convention too?]

> On Thu, 15 Jan 2009, Sitaram Chamarty wrote:

>> I'm unable to make "rebase -p" carry an evil merge over.
>> The "evil" part stays behind.
>> 
>> I'm not sure if that is intentional or not, or (more likely)
>> my brain has become addled and I missed something somewhere.
>
> Yes, this is intentional.
>
> 	Instead of ignoring merges, try to recreate them.
>
> That means it tries to recreate them.  Not that it is successful.  And not 
> even that it realizes when it failed.

Is a conflicted merge that was resolved by making a change
that was in neither parent, an evil merge?

Regardless, I suspect rebase -p will not be able to carry
such a merge over.

But if it won't carry over the evil in an evil merge, what
other uses are there for "rebase -p" as opposed to rebase?
Seems to me that the DAG may be different but the tree you
end up with is the same then.

I'm sure someone has a blog post or a bookmark or an article
or something they wrote long ago about "rebase -i -p".
Anyone?

^ permalink raw reply

* Re: [PATCH] checkout: implement "-" shortcut name for last branch
From: Johannes Schindelin @ 2009-01-15 18:34 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Junio C Hamano
In-Reply-To: <200901151805.44747.trast@student.ethz.ch>

Hi,

On Thu, 15 Jan 2009, Thomas Rast wrote:

> Johannes Schindelin wrote:
> > There are a number of issues why I would like to avoid introducing 
> > LAST_HEAD:
> > 
> > - it does not work when you are using different Git versions on the same 
> >   repository,
> > 
> > - it does not work when you switched recently,
> 
> If you switch once, you'll be able to use the feature one checkout
> later than if it was reflog-based.
> 
> If you switch a lot, the feature won't be in your git half the time
> anyway.

But once it is, you could also have something like "git checkout -{5}" 
meaning the 5th last branch you were on.

No, I am not married to that syntax

> > - you are storing redundant information,
> 
> AFAIK it's the first instance of this data in a non-free-form field.
> There's also the precedent of ORIG_HEAD.

See below.

> > - yes, the field is meant for user consumption, but no, it is not 
> >   free-form,
> 
> It's a field of almost arbitrary character data, filled by 70% of the
> update-ref calls I can find in git.git in a "<tool>: <comment>" format
> and by the rest with things such as "initial pull" or
> "refs/remotes/git-svn: updating HEAD".  (The latter is so informative
> that it probably deserves a fix.)  How is that not free-form?

That is not free-form, as the "<tool>:" is a hard convention all obey 
(and therefore, git checkout - only relies on _checkout_ not changing the 
format), and checkout is sufficiently plumbing that we will not change it 
all that lightly, certainly not when "git checkout -" depends on it.

So I think that those free-form concerns are totally unfounded.

Oh, and before you say that people could mess with GIT_REFLOG_ACTION, git 
checkout is no longer a script, and creates the message itself.  So we 
have full control over it.

They could edit the logs directly, but that applies to virtually the whole 
repository, and can safely be ignored as a lemming behavior.

> > - AFAICT your version could never be convinced to resurrect deleted 
> >   branches, without resorting to reflogs anyway.
> 
> Neither can any other use of git-checkout without the user manually
> recovering some valid revspec referring to the old branch tip from the
> reflog.

To the contrary.  The reflog has this information together with the 
message "moved from ...".

> > - the reflog method reflects pretty much exactly how people work around 
> >   the lack of "checkout -" currently, so why not just use the same proven 
> >   approach?
> 
> So you can make me fight an uphill battle against your idea how it
> should be done.

If you can convince me that there are benefits from introducing yet 
another file in $GIT_DIR and duplicating information that is in the 
reflogs already, then no, it's not an uphill battle.

I mean, I _like_ the feature.  Otherwise I would not spend so much time 
suggesting what I think would be a method more in line with what we have 
already.

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Johan Herland @ 2009-01-15 18:46 UTC (permalink / raw)
  To: git
  Cc: Johannes Schindelin, Junio C Hamano, Sverre Rabbelier,
	Johannes Sixt, Anders Melchiorsen
In-Reply-To: <alpine.DEB.1.00.0901151921040.3586@pacific.mpi-cbg.de>

On Thursday 15 January 2009, Johannes Schindelin wrote:
> On Thu, 15 Jan 2009, Junio C Hamano wrote:
> > 'stop' would be closest to what it currently does.  It stops and it
> > is up to you how to screw up the result ;-).
>
> But it shares the first letter with 'squash'.

Personally, I'd rather use "pause", but that is taken as well.

Other suggestions:

wait
yield
rest
timeout


..Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Anders Melchiorsen @ 2009-01-15 18:53 UTC (permalink / raw)
  To: Johan Herland
  Cc: git, Johannes Schindelin, Junio C Hamano, Sverre Rabbelier,
	Johannes Sixt
In-Reply-To: <200901151946.04991.johan@herland.net>

Johan Herland <johan@herland.net> writes:

> On Thursday 15 January 2009, Johannes Schindelin wrote:
>> On Thu, 15 Jan 2009, Junio C Hamano wrote:
>> > 'stop' would be closest to what it currently does.  It stops and it
>> > is up to you how to screw up the result ;-).
>>
>> But it shares the first letter with 'squash'.
>
> Personally, I'd rather use "pause", but that is taken as well.
>
> Other suggestions:
>
> wait
> yield
> rest
> timeout

break



Anders.

^ permalink raw reply

* Re: [PATCH take 3 0/4] color-words improvements
From: Johannes Schindelin @ 2009-01-15 19:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Teemu Likonen, Thomas Rast, git, Santi Béjar
In-Reply-To: <7vmydstoys.fsf@gitster.siamese.dyndns.org>

Hi,

On Thu, 15 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> I didn't see the semantics of color-words documented in the original
> either,

Yeah, my bad.  Will try to fix it with this round of patches.

Actually, I'll give a quick outline right here:

Idea: the idea of word diff is to show the differences on a word level 
instead of line level.  To make it easier for humans (albeit we studiously 
exclude color blinds with our defaults), we do not show "+" and "-" as the 
standard diff does, but use colors to designate if the words were removed 
or added.

Now, the thing is that the inter-word parts _can_ differ.  The idea here 
is to show the part of the postimage and drop the preimage under the 
table.

Method: We use libxdiff as the real workhorse.  First, we let it generate 
a line diff.

Then we reconstruct the preimage and postimage for each hunk, process both 
into new images that have at most one word (in the new code exactly one 
word) per line, and feed the new preimage/postimage pair to libxdiff.

>From the output of libxdiff, we reconstruct which words were actually 
removed and which were added.  Then -- like the line based diff -- we 
combine the runs of common words, removed words and added words, and show 
them.

The algorithm I implemented in the new patch series is actually much 
cleaner than the old one:

- it feeds images to libxdiff which contain _exactly_ one word per line, 
  decoupling the word offsets in the original image from the offsets in 
  the processed image,

- this decoupling allows for arbitrary word boundaries, even 0-character 
  ones,

- it parses the hunk headers of the libxdiff output instead of the "-", 
  "+" and " " lines, and therefore does not have to play tricks with the 
  newline character in the middle of a run of removed words.

> What happens if a portion of background is only in the preimage?

If it is in a run of words that were removed, i.e. that are only in the 
preimage, then it is shown in that part.  Otherwise, the background of the 
preimage is never shown.

> E.g. when these two are compared:
> 
>   bbb aaa bb aa b
>   ccc aaa cc
> 
> what should happen?  We would want to say "aa" was removed by showing it
> in red, but on what background should it be displayed?  cc <red>aa</red>
> b?

If we are only ever interested in the 'a's, I'd say that the output should 
only reflect that.  In other words, what the current code does (ccc 
aaa<red>aa</red> cc) is okay IMHO.  After all, we said we're interested in 
the 'a's, so we should not complain that it did not show us the removal of 
'b's.

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Johannes Schindelin @ 2009-01-15 19:28 UTC (permalink / raw)
  To: Anders Melchiorsen
  Cc: Johan Herland, git, Junio C Hamano, Sverre Rabbelier,
	Johannes Sixt
In-Reply-To: <87ocy8flka.fsf@cup.kalibalik.dk>

Hi,

On Thu, 15 Jan 2009, Anders Melchiorsen wrote:

> break

As rebase -i is just a loop of cherry-picks, as a C programmer I would 
think: "does this break out of the loop"?

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC PATCH] Make the rebase edit mode really end up in an edit state
From: Wincent Colaiuta @ 2009-01-15 19:27 UTC (permalink / raw)
  To: Johan Herland
  Cc: git, Johannes Schindelin, Junio C Hamano, Sverre Rabbelier,
	Johannes Sixt, Anders Melchiorsen
In-Reply-To: <200901151946.04991.johan@herland.net>

El 15/1/2009, a las 19:46, Johan Herland escribió:

> On Thursday 15 January 2009, Johannes Schindelin wrote:
>> On Thu, 15 Jan 2009, Junio C Hamano wrote:
>>> 'stop' would be closest to what it currently does.  It stops and it
>>> is up to you how to screw up the result ;-).
>>
>> But it shares the first letter with 'squash'.
>
> Personally, I'd rather use "pause", but that is taken as well.
>
> Other suggestions:
>
> wait
> yield
> rest
> timeout

Perhaps stating the obvious, but:

wait - best suggestion so far, seeing as we can't use "stop"

yield - might sound intuitive to a Ruby programmer; but for others  
it's probably not so obvious as "yield" has a number of meanings in  
normal English similar to "give up", "give over" etc

rest - not quite as good as "wait"; machines wait for humans, but the  
never need to rest

timeout - sounds like an error condition, so not really appropriate

Sorry for participating in the painting. Just thought that "wait" was  
good enough to merit some positive feedback.

Cheers,
Wincent

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2009, #03; Wed, 14)
From: Kirill Smelkov @ 2009-01-15 19:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmydu3yy7.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 14, 2009 at 03:32:32AM -0800, Junio C Hamano wrote:

[...]

> [Actively cooking]
> 
> * gb/gitweb-opml (Fri Jan 2 13:49:30 2009 +0100) 2 commits
>  + gitweb: suggest name for OPML view
>  + gitweb: don't use pathinfo for global actions
> 
> * ks/maint-mailinfo-folded (Mon Jan 12 15:22:11 2009 -0800) 2 commits
>  - mailinfo: 'From:' header should be unfold as well
>  - mailinfo: correctly handle multiline 'Subject:' header
> 
> The author seems to have more updates, but I couldn't extract them from
> the e-mail.

Sorry for the inconvenience, and please pull them all from

    git://repo.or.cz/git/kirr.git   for-junio


Kirill Smelkov (4):
      mailinfo: 'From:' header should be unfold as well
      mailinfo: more smarter removal of rfc822 comments from 'From'
      mailinfo: correctly handle multiline 'Subject:' header
      mailinfo: add explicit test for mails like '<a.u.thor@example.com> (A U Thor)'


Thanks,
Kirill

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox