Git development
 help / color / mirror / Atom feed
* [PATCH] Makes 'git-stash show' stay quiet when there are no stashes.
From: Jing Xue @ 2007-12-14  1:28 UTC (permalink / raw)
  To: git


(I tried to send this trivial patch for a couple of times using
git-send-email, but somehow it never turned up.)

Currently when there are no stashes, 'git stash show' basically aborts with an
error message from rev-parse: "fatal: Needed a single revision", which can be
confusing. This patch makes git-stash keep quiet and exit gracefully in that
case.

---
 git-stash.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..dbdaeaf 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -119,6 +119,10 @@ show_stash () {
 		flags=--stat
 	fi
 	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
+	if test -z "$s"
+	then
+		exit 0
+	fi
 
 	w_commit=$(git rev-parse --verify "$s") &&
 	b_commit=$(git rev-parse --verify "$s^") &&
-- 
1.5.4.rc0.1.g3696

^ permalink raw reply related

* Re: Something is broken in repack
From: Jakub Narebski @ 2007-12-14  1:04 UTC (permalink / raw)
  To: git; +Cc: gcc
In-Reply-To: <47616044.7070504@viscovery.net>

Johannes Sixt wrote:
> Paolo Bonzini schrieb:
>> Nguyen Thai Ngoc Duy wrote:
>>>
>>> Is there an alternative to "git repack -a -d" that repacks everything
>>> but the first pack?
>> 
>> That would be a pretty good idea for big repositories.  If I were to
>> implement it, I would actually add a .git/config option like
>> pack.permanent so that more than one pack could be made permanent; then
>> to repack really really everything you'd need "git repack -a -a -d".
> 
> It's already there: If you have a pack .git/objects/pack/pack-foo.pack, then
> "touch .git/objects/pack/pack-foo.keep" marks the pack as precious.

Actually you can (and probably should) put the one line with the _reason_
pack is to be kept in the *.keep file.

Hmmm... it is even documented in git-gc(1)... and git-index-pack(1) of
all things.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 5/5] Add tests for "git diff --check" with core.whitespace options
From: Junio C Hamano @ 2007-12-14  0:55 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: git, gitster
In-Reply-To: <1197552751-53480-6-git-send-email-win@wincent.com>

Wincent Colaiuta <win@wincent.com> writes:

> +test_expect_success 'check spaces as indentation (indent-with-non-tab: off)' '
> +
> +	git config core.whitespace "-indent-with-non-tab"
> +	echo "                foo ();" > x &&
> +	git diff --check
> +
> +'
> +
> +test_expect_failure 'check spaces as indentation (indent-with-non-tab: on)' '
> +
> +	git config core.whitespace "indent-with-non-tab" &&
> +	echo "                foo ();" > x &&
> +	git diff --check
> +
> +'
> +
>  test_done

It is good to check both positive and negative cases, but I would prefer
avoiding test_expect_failure.  You cannot tell if the "git config"
before the real test failed or "git diff --check" correctly reported
whitespace breakage --- both would cause the test to pass.

^ permalink raw reply

* Re: [PATCH 2/5] New version of pre-commit hook
From: Jakub Narebski @ 2007-12-14  0:17 UTC (permalink / raw)
  To: git
In-Reply-To: <1197552751-53480-3-git-send-email-win@wincent.com>

Wincent Colaiuta wrote:

> Now that "git diff --check" indicates problems with its exit code the
> pre-commit hook becomes a trivial one-liner.

> -         if (/^(?:[<>=]){7}/) {
> -             bad_line("unresolved merge conflict", $_);
> -         }

Aren't you losing this check with rewrite?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH 4/5] Make "diff --check" output match "git apply"
From: Junio C Hamano @ 2007-12-14  0:12 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: git
In-Reply-To: <1197552751-53480-5-git-send-email-win@wincent.com>

Nice attention to the details.

^ permalink raw reply

* Re: [egit] How-to use egit
From: Wink Saville @ 2007-12-14  0:11 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200712140024.20667.robin.rosenberg.lists@dewire.com>

Robin Rosenberg wrote:
> torsdag 13 december 2007 skrev Wink Saville:
>   
>> Yes, I selected them all and a single jar was created which
>>     
> Never seen that behaviour. I'm surprised that would be the default.
> When I export I get separate jars.
>
>   
>> I then started eclipse and now I see: Window/Preferences/Team/Git
>> When I select "Git" in the above, I see "Hi, I'm an empty preference page."
>>     
> That's all there is for now, but it means the plugin is activated.
>
>   
>> But again, I don't see any new menu items that
>> would allow me to look at history or make commits.
>> Is there more I need to to install the plugin, or
>> maybe I just need a little guidance on how to use it.
>>     
>
> We do not actually implement any menus in the menu bar. The only menus
> are in the context (right click) menu on resources.
>
> Try right-click on a project in the project explorer and the Team menu>Share project.
>
> -- robin
>   
Got it:

One thing that would be helpful for me is the
relationship between the items in the context menu
and the git commands, it isn't obvious to me.

As a for instance, it seems "Update index (Refresh)"
is a "git add <file>" but what is "Assume unchanged"?

One other thing is there a "git diff" and or "git diff HEAD"?

Thanks,

Wink

^ permalink raw reply

* Re: [PATCH 3/5] Unify whitespace checking
From: Junio C Hamano @ 2007-12-14  0:03 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: git, gitster
In-Reply-To: <1197552751-53480-4-git-send-email-win@wincent.com>

Wincent Colaiuta <win@wincent.com> writes:

> The new function is called check_and_emit_line() and it does two things:
> checks a line for whitespace errors and optionally emits it. The checking
> is based on lines of content rather than patch lines (in other words, the
> caller must strip the leading "+" or "-"); this was suggested by Junio on
> the mailing list to allow for a future extension to "git show" to display
> whitespace errors in blobs.

I do not think "git show" is a realistic "future extension", by the
way.  But at least from the interface-cleanliness point of view, I think
not passing the leading "+" to the function is a right thing to do.

> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index dc538b3..a0a47dd 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -121,7 +121,7 @@ test_expect_success 'check mixed spaces and tabs in indent' '
>  
>  	# This is indented with SP HT SP.
>  	echo " 	 foo();" > x &&
> -	git diff --check | grep "space before tab"
> +	git diff --check | grep "Space in indent is followed by a tab"
>  
>  '

Hmph.  I think with the multiple detection this rewording would make the
error message very long to read.  Was the rewording really necessary?

> +/* If stream is non-NULL, emits the line after checking. */
> +unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
> +                             FILE *stream, const char *set,
> +                             const char *reset, const char *ws)
> +{

Honestly, I regretted suggesting this, fearing that it might make the
checking too costly, but the code is clean, readable, and does not look
costly at all.  Nice job.

^ permalink raw reply

* Re: [PATCH 1/5] "diff --check" should affect exit status
From: Junio C Hamano @ 2007-12-13 23:51 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: git
In-Reply-To: <1197552751-53480-2-git-send-email-win@wincent.com>

Wincent Colaiuta <win@wincent.com> writes:

> @@ -2965,8 +2976,8 @@ void diff_flush(struct diff_options *options)
>  			     DIFF_FORMAT_CHECKDIFF)) {
>  		for (i = 0; i < q->nr; i++) {
>  			struct diff_filepair *p = q->queue[i];
> -			if (check_pair_status(p))
> -				flush_one_pair(p, options);
> +			if (check_pair_status(p) && flush_one_pair(p, options))
> +				DIFF_OPT_SET(options, CHECK_FAILED);
>  		}
>  		separator++;
>  	}

Isn't this wrong when check is not in effect?

I think highjacking the "did we encounter problems" return value of the
entire callchain for the purpose of checkdiff is very ugly and wrong to
begin with, so please do not argue "but if checkdiff is not in effect,
the caller does not check CHECK_FAILED".

Wouldn't it be much cleaner to make diff_flush_checkdiff(), or its
underlying function run_checkdiff(), set that CHECK_FAILED flag to
options structure, and return success?  The toplevel caller can decide
to exit with non-zero when --check is in effect and CHECK_FAILED flag is
set.

^ permalink raw reply

* Re: [egit] How-to use egit
From: Robin Rosenberg @ 2007-12-13 23:24 UTC (permalink / raw)
  To: Wink Saville; +Cc: git
In-Reply-To: <4761B3B2.4040807@saville.com>

torsdag 13 december 2007 skrev Wink Saville:
> Yes, I selected them all and a single jar was created which
Never seen that behaviour. I'm surprised that would be the default.
When I export I get separate jars.

> I then started eclipse and now I see: Window/Preferences/Team/Git
> When I select "Git" in the above, I see "Hi, I'm an empty preference page."
That's all there is for now, but it means the plugin is activated.

> But again, I don't see any new menu items that
> would allow me to look at history or make commits.
> Is there more I need to to install the plugin, or
> maybe I just need a little guidance on how to use it.

We do not actually implement any menus in the menu bar. The only menus
are in the context (right click) menu on resources.

Try right-click on a project in the project explorer and the Team menu>Share project.

-- robin

^ permalink raw reply

* Re: Using git with Eclipse
From: Wink Saville @ 2007-12-13 22:41 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: Shawn O. Pearce, git
In-Reply-To: <200712132223.31109.robin.rosenberg.lists@dewire.com>

Robin Rosenberg wrote:
> onsdag 12 december 2007 skrev Wink Saville:
>   
>> Robin Rosenberg wrote:
>>     
>>> tisdag 11 december 2007 skrev Wink Saville:
>>>   
>>> <snip>
>>> -- robin
>>>   
>>>       
>> I case I'll take the bait and a guinea pig:)
>> Please send me a link to what you'd like me to test.
>>     
>
> Great! Please start with http://repo.or.cz/w/egit.git and test the INSTALL
> instructions first. We do not provide an updatesite or prebuilt plugins yet.
>
> -- robin
>
>   
Robin, I'm having a little trouble following the install instructions
due to my inexperience with Eclipse. Anyway see my "[egit] How-to use egit".
I might have it installed correctly now, but not sure how to use,
I just made a reply to Jing with some details.

Cheers,

Wink

^ permalink raw reply

* Re: [egit] How-to use egit
From: Wink Saville @ 2007-12-13 22:35 UTC (permalink / raw)
  To: git
In-Reply-To: <20071213024735.GB7492@fawkes>

Jing Xue wrote:
> On Wed, Dec 12, 2007 at 11:12:34AM -0800, Wink Saville wrote:
>   
>> Robin et. al,
>>
>>
>> I cloned egit from git://repo.or.cz/egit and imported it into my workspace
>> following the instructions in INSTALL I did successfully compile although
>> there were 1115 warnings but no compilation errors.
>>
>> I then exported to a archive org.spearce.egit.jar and then copied this file
>> to /usr/eclipse/plugin/ I then restarted Eclipse. But I don't see any 
>> evidence
>>     
>
> I have five jars:
>
> /opt/eclipse/plugins$ ll org.spearce*
> -rw-r----- 1 jingxue jingxue 2.5K 2007-12-01 20:45 org.spearce.egit_0.3.0.200712020145.jar
> -rw-r----- 1 jingxue jingxue  70K 2007-12-01 20:45 org.spearce.egit.core_0.3.0.200712020145.jar
> -rw-r----- 1 jingxue jingxue  14K 2007-12-01 20:45 org.spearce.egit.core.test_0.3.0.200712020145.jar
> -rw-r----- 1 jingxue jingxue 107K 2007-12-01 20:45 org.spearce.egit.ui_0.3.0.200712020145.jar
> -rw-r----- 1 jingxue jingxue 223K 2007-12-01 20:45 org.spearce.jgit_0.3.0.200712020145.jar
>
> Did you select all the org.spearce projects before exporting?
>
>   
Yes, I selected them all and a single jar was created which
I put in /usr/eclipse/plughins, which apparently was a mistake.
I've now unzipped the jar and placed the 5 jars in the plugin directory:

wink@ic2d1:$ pwd
/usr/eclipse/plugins
wink@ic2d1:$ ls -l *spearce*
-rw-r--r-- 1 root root   2506 2007-12-13 13:54 
org.spearce.egit_0.3.0.200712121008.jar
-rw-r--r-- 1 root root  71396 2007-12-13 13:54 
org.spearce.egit.core_0.3.0.200712121008.jar
-rw-r--r-- 1 root root  13899 2007-12-13 13:54 
org.spearce.egit.core.test_0.3.0.200712121008.jar
-rw-r--r-- 1 root root 108661 2007-12-13 13:54 
org.spearce.egit.ui_0.3.0.200712121008.jar
-rw-r--r-- 1 root root 227877 2007-12-13 13:54 
org.spearce.jgit_0.3.0.200712121008.jar

I then started eclipse and now I see: Window/Preferences/Team/Git
When I select "Git" in the above, I see "Hi, I'm an empty preference page."

Now what; currently I have my entire workspace in git:

wink@ic2d1:$ ls -al
total 36
drwxr-xr-x  9 wink wink 4096 2007-12-12 14:54 .
drwxr-xr-x 69 wink wink 4096 2007-12-13 14:22 ..
drwxr-xr-x  9 wink wink 4096 2007-12-12 15:09 android
drwxr-xr-x  3 wink wink 4096 2007-12-10 21:58 com
drwxr-xr-x  8 wink wink 4096 2007-12-13 14:21 .git
drwxr-xr-x  3 wink wink 4096 2007-12-12 10:45 .metadata
drwxr-xr-x  4 wink wink 4096 2007-12-10 21:58 test0
drwxr-xr-x  2 wink wink 4096 2007-12-10 21:58 test1
drwxr-xr-x  5 wink wink 4096 2007-12-13 14:22 testStdJavaDebug

But how do I tell the plugin where my repo is?
I also tried making a git repo in testStdJavaDebug:

wink@ic2d1:$ ls -al
total 28
drwxr-xr-x 5 wink wink 4096 2007-12-13 14:22 .
drwxr-xr-x 9 wink wink 4096 2007-12-12 14:54 ..
drwxr-xr-x 3 wink wink 4096 2007-12-10 22:03 bin
-rw-r--r-- 1 wink wink  226 2007-12-10 21:58 .classpath
drwxr-xr-x 8 wink wink 4096 2007-12-13 14:22 .git
-rw-r--r-- 1 wink wink  375 2007-12-10 21:58 .project
drwxr-xr-x 3 wink wink 4096 2007-12-10 21:58 src

But again, I don't see any new menu items that
would allow me to look at history or make commits.
Is there more I need to to install the plugin, or
maybe I just need a little guidance on how to use it.

Thanks,

Wink

^ permalink raw reply

* [PATCH 2/2] xdi_diff: trim common trailing lines
From: Junio C Hamano @ 2007-12-13 22:31 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.0.99999.0712122219160.20487@xanadu.home>

This implements earlier Linus's optimization to trim common lines at the
end before passing them down to low level xdiff interface for all of our
xdiff users.

We could later enhance this to also trim common leading lines, but that
would need tweaking of the output function to add the number of lines
trimmed at the beginning to line numbers that appear in the hunk
headers.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 xdiff-interface.c |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 69a022c..f2cd488 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -103,9 +103,41 @@ int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 	return 0;
 }
 
+/*
+ * Trim down common substring at the end of the buffers,
+ * but leave at least ctx lines at the end.
+ */
+static void trim_common_tail(mmfile_t *a, mmfile_t *b, int ctx)
+{
+	const int blk = 1024;
+	long trimmed = 0, recovered = 0;
+	int i;
+	char *ap = a->ptr + a->size;
+	char *bp = b->ptr + b->size;
+	long smaller = (a->size < b->size) ? a->size : b->size;
+
+	while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
+		trimmed += blk;
+		ap -= blk;
+		bp -= blk;
+	}
+
+	for (i = 0, recovered = 0; recovered < trimmed && i <= ctx; i++) {
+		while (recovered < trimmed && ap[recovered] != '\n')
+			recovered++;
+	}
+	a->size -= (trimmed - recovered);
+	b->size -= (trimmed - recovered);
+}
+
 int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *xecb)
 {
-	return xdl_diff(mf1, mf2, xpp, xecfg, xecb);
+	mmfile_t a = *mf1;
+	mmfile_t b = *mf2;
+
+	trim_common_tail(&a, &b, xecfg->ctxlen);
+
+	return xdl_diff(&a, &b, xpp, xecfg, xecb);
 }
 
 int read_mmfile(mmfile_t *ptr, const char *filename)
-- 
1.5.4.rc0.1.g37d0

^ permalink raw reply related

* [PATCH 1/2] xdl_diff: identify call sites.
From: Junio C Hamano @ 2007-12-13 22:31 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.0.99999.0712122219160.20487@xanadu.home>

This inserts a new function xdi_diff() that currently does not
do anything other than calling the underlying xdl_diff() to the
callchain of current callers of xdl_diff() function.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Nicolas Pitre <nico@cam.org> writes:

 > On Wed, 12 Dec 2007, Junio C Hamano wrote:
 >
 >> Here are the topics that have been cooking.
 >
 > What about the blame speedup patch from Linus (Message-ID: 
 > <alpine.LFD.0.9999.0712111548200.25032@woody.linux-foundation.org>)

 I would prefer to do a bit more generic solution, not a special hack for
 speeding up blame on prepend-only files, with a proper log message.

 Here is the first installment of such.

 builtin-blame.c   |    2 +-
 builtin-rerere.c  |    2 +-
 combine-diff.c    |    2 +-
 diff.c            |   10 +++++-----
 merge-file.c      |    2 +-
 merge-tree.c      |    2 +-
 xdiff-interface.c |    5 +++++
 xdiff-interface.h |    1 +
 8 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 5466d01..99ea0a0 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -542,7 +542,7 @@ static struct patch *compare_buffer(mmfile_t *file_p, mmfile_t *file_o,
 	state.ret->chunks = NULL;
 	state.ret->num = 0;
 
-	xdl_diff(file_p, file_o, &xpp, &xecfg, &ecb);
+	xdi_diff(file_p, file_o, &xpp, &xecfg, &ecb);
 
 	if (state.ret->num) {
 		struct chunk *chunk;
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 7449323..37e6248 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -260,7 +260,7 @@ static int diff_two(const char *file1, const char *label1,
 	memset(&xecfg, 0, sizeof(xecfg));
 	xecfg.ctxlen = 3;
 	ecb.outf = outf;
-	xdl_diff(&minus, &plus, &xpp, &xecfg, &ecb);
+	xdi_diff(&minus, &plus, &xpp, &xecfg, &ecb);
 
 	free(minus.ptr);
 	free(plus.ptr);
diff --git a/combine-diff.c b/combine-diff.c
index 5a658dc..e22db89 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -226,7 +226,7 @@ static void combine_diff(const unsigned char *parent, mmfile_t *result_file,
 	state.num_parent = num_parent;
 	state.n = n;
 
-	xdl_diff(&parent_file, result_file, &xpp, &xecfg, &ecb);
+	xdi_diff(&parent_file, result_file, &xpp, &xecfg, &ecb);
 	free(parent_file.ptr);
 
 	/* Assign line numbers for this parent.
diff --git a/diff.c b/diff.c
index 9c79ee2..3dd2f35 100644
--- a/diff.c
+++ b/diff.c
@@ -439,7 +439,7 @@ static void diff_words_show(struct diff_words_data *diff_words)
 	ecb.outf = xdiff_outf;
 	ecb.priv = diff_words;
 	diff_words->xm.consume = fn_out_diff_words_aux;
-	xdl_diff(&minus, &plus, &xpp, &xecfg, &ecb);
+	xdi_diff(&minus, &plus, &xpp, &xecfg, &ecb);
 
 	free(minus.ptr);
 	free(plus.ptr);
@@ -1393,7 +1393,7 @@ static void builtin_diff(const char *name_a,
 		if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
 			ecbdata.diff_words =
 				xcalloc(1, sizeof(struct diff_words_data));
-		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
+		xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 		if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
 			free_diff_words_data(&ecbdata);
 	}
@@ -1446,7 +1446,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
 		xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
 		ecb.outf = xdiff_outf;
 		ecb.priv = diffstat;
-		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
+		xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 	}
 
  free_and_return:
@@ -1486,7 +1486,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
 		xpp.flags = XDF_NEED_MINIMAL;
 		ecb.outf = xdiff_outf;
 		ecb.priv = &data;
-		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
+		xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 	}
  free_and_return:
 	diff_free_filespec_data(one);
@@ -2898,7 +2898,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
 		ecb.outf = xdiff_outf;
 		ecb.priv = &data;
-		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
+		xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 	}
 
 	SHA1_Final(sha1, &ctx);
diff --git a/merge-file.c b/merge-file.c
index 1e031ea..2a939c9 100644
--- a/merge-file.c
+++ b/merge-file.c
@@ -71,7 +71,7 @@ static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
 	res->size = 0;
 
 	ecb.priv = res;
-	return xdl_diff(f1, f2, &xpp, &xecfg, &ecb);
+	return xdi_diff(f1, f2, &xpp, &xecfg, &ecb);
 }
 
 void *merge_file(struct blob *base, struct blob *our, struct blob *their, unsigned long *size)
diff --git a/merge-tree.c b/merge-tree.c
index 7d4f628..e083246 100644
--- a/merge-tree.c
+++ b/merge-tree.c
@@ -119,7 +119,7 @@ static void show_diff(struct merge_list *entry)
 	if (!dst.ptr)
 		size = 0;
 	dst.size = size;
-	xdl_diff(&src, &dst, &xpp, &xecfg, &ecb);
+	xdi_diff(&src, &dst, &xpp, &xecfg, &ecb);
 	free(src.ptr);
 	free(dst.ptr);
 }
diff --git a/xdiff-interface.c b/xdiff-interface.c
index be866d1..69a022c 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -103,6 +103,11 @@ int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 	return 0;
 }
 
+int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *xecb)
+{
+	return xdl_diff(mf1, mf2, xpp, xecfg, xecb);
+}
+
 int read_mmfile(mmfile_t *ptr, const char *filename)
 {
 	struct stat st;
diff --git a/xdiff-interface.h b/xdiff-interface.h
index fb742db..f7f791d 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -13,6 +13,7 @@ struct xdiff_emit_state {
 	unsigned long remainder_size;
 };
 
+int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb);
 int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf);
 int parse_hunk_header(char *line, int len,
 		      int *ob, int *on,
-- 
1.5.4.rc0.1.g37d0

^ permalink raw reply related

* Re: git merge --no-commit <branch>; does commit
From: Michael Dressel @ 2007-12-13 21:28 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Michael Dressel, git
In-Reply-To: <81b0412b0712131319h63609810m593f0e552d02a83c@mail.gmail.com>



On Thu, 13 Dec 2007, Alex Riesen wrote:

> On 13/12/2007, Michael Dressel <MichaelTiloDressel@t-online.de> wrote:
>> git merge --no-commit <branch> does "create" a commit. At lesat the
>> head and index are moved to the new commit fetched from <branch>. Maybe
>> that is because git was able to do a fast forward?
>
> Yes. Because fast-forward is what it called: fast-forward.
> It does not do any commits at all.
>

It looks like I misunderstood the meaning of --no-commit. I have 
to use --squash in this case.

Cheers,
Michael

^ permalink raw reply

* Re: git merge --no-commit <branch>; does commit
From: Alex Riesen @ 2007-12-13 21:23 UTC (permalink / raw)
  To: Michael Dressel; +Cc: git
In-Reply-To: <81b0412b0712131319h63609810m593f0e552d02a83c@mail.gmail.com>

On 13/12/2007, Alex Riesen <raa.lkml@gmail.com> wrote:
> On 13/12/2007, Michael Dressel <MichaelTiloDressel@t-online.de> wrote:
> > git merge --no-commit <branch> does "create" a commit. At lesat the
> > head and index are moved to the new commit fetched from <branch>. Maybe
> > that is because git was able to do a fast forward?
>
> Yes. Because fast-forward is what it called: fast-forward.
> It does not do any commits at all.
>

If you have a very recent git, you can avoid fast-forward when merging
by running with --no-ff. It has it's problems in general case, so it is not
default (see the git ml archive for "fast-forward does not commit").

^ permalink raw reply

* Re: Using git with Eclipse
From: Robin Rosenberg @ 2007-12-13 21:23 UTC (permalink / raw)
  To: Wink Saville; +Cc: Shawn O. Pearce, git
In-Reply-To: <475F7A94.4000209@saville.com>

onsdag 12 december 2007 skrev Wink Saville:
> Robin Rosenberg wrote:
> > tisdag 11 december 2007 skrev Wink Saville:
> >   
> > <snip>
> > -- robin
> >   
> 
> I case I'll take the bait and a guinea pig:)
> Please send me a link to what you'd like me to test.

Great! Please start with http://repo.or.cz/w/egit.git and test the INSTALL
instructions first. We do not provide an updatesite or prebuilt plugins yet.

-- robin

^ permalink raw reply

* Re: git merge --no-commit <branch>; does commit
From: Alex Riesen @ 2007-12-13 21:19 UTC (permalink / raw)
  To: Michael Dressel; +Cc: git
In-Reply-To: <alpine.LSU.0.99999.0712132151080.5326@castor.milkiway.cos>

On 13/12/2007, Michael Dressel <MichaelTiloDressel@t-online.de> wrote:
> git merge --no-commit <branch> does "create" a commit. At lesat the
> head and index are moved to the new commit fetched from <branch>. Maybe
> that is because git was able to do a fast forward?

Yes. Because fast-forward is what it called: fast-forward.
It does not do any commits at all.

^ permalink raw reply

* git merge --no-commit <branch>; does commit
From: Michael Dressel @ 2007-12-13 21:01 UTC (permalink / raw)
  To: git

Hi,

git merge --no-commit <branch> does "create" a commit. At lesat the 
head and index are moved to the new commit fetched from <branch>. Maybe 
that is because git was able to do a fast forward?

Cheers,
Michael

^ permalink raw reply

* Re: [RFH] convert shortlog to use parse_options
From: Pierre Habouzit @ 2007-12-13 20:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vzlwe1jeb.fsf@gitster.siamese.dyndns.org>

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

On Thu, Dec 13, 2007 at 07:31:24PM +0000, Junio C Hamano wrote:
> So I am not entirely opposed to your version, nor I am claiming my
> suggestion is better.  However, I just thought that "some parameters you
> MUST stick to the flag" might create confusion to the end users, and I
> wanted to see if others can come up with a less confusing alternative.
> And the way I did so was to keep the discussion going by stirring the
> pot a bit.

I understand that, and I hope I wasn't sounding harsh at all, I was just
debating. Note that I don't like the asymmetry of some options needing a
specific syntax whereas all the rest is lax. It's cumbersome at the very
least. Though, to me there is one gain: when the user uses the
--long-opt=foo version you are _sure_ about what he meant. When you have
--long-opt foo you're not.

Your proposal tries hard to do what the user meant, with a reasonable
chance of being wrong. My proposal is on the conservative side, but
generate totally incomprehensible errors: if you do
  $ git describe --abbrev 10 HEAD
with my patch you get a not very nice
  fatal: Not a valid object name 10
as an answer. which is kind of going back to the kind of situations
parse-options is trying to avoid in the first place. I wouldn't be
really annoyed by my solution if we were able to generate an
intelligible error message instead. But guess what, if we were able to
do so, we would be able to fix the ambiguity in the first place *sigh*.


So maybe the thing would be to sleep on a it a bit and not taking any of
my patches yet (not even in pu) and let people think. It's not a major
problem, though it's one that must be fixed before 1.5.4 because we
don't want a broken option parser ever be released.


Note that there is another path, that would be to disallow mixing of
options with real arguments, and have an option separator (though with
`--` having a distinct meaning in git, that wouldn't be that easy). But
sadly some git commands already allowed such a thing, so imposing it in
parse-options would be backward incompatible for those. And I believe
many people are attached to this feature anyways.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: [RFH] convert shortlog to use parse_options
From: Junio C Hamano @ 2007-12-13 20:46 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: Pierre Habouzit, Jeff King, git
In-Reply-To: <1197571656.28742.13.camel@hinata.boston.redhat.com>

Kristian Høgsberg <krh@redhat.com> writes:

> Oops, sorry about that.  I just wanted to say we shouldn't jump through
> all these hoops to make the option parser support every type of option
> there ever was in the git command line ui.  A lot of these were probably
> decided somewhat arbitrarily by whoever implemented the command.
> Instead it's an opportunity to retroactively enforce some consistency
> and predictability to the various option-styles that have been
> hand-rolled over time in different git commands.

That principle is fine, but I do not think it is relevant to what is
being discussed.  The issue is what to do with a flag that can
optionally take a parameter but works fine without because it has a
default.

 * You can obviously disallow such a flag, and call the result
   "consistent".  I do not think we want to go that route.  --abbrev, -M
   (to diff) and -w (to shortlog) are good examples why this is a good
   thing.  You want to use the default most of the time, but want to be
   able to tweak the value sometimes.

 * You can alternatively require parameters to such a flag always stuck
   with the flag, which is what Pierre did.  I suspected that is
   introducing an inconsistency and may be confusing to the users ("I
   can write -n=1 or -n 2, but why not --abbrev 7???") and wanted to see
   if somebody can come up with a better alternative.

 * You can try to make the parser a bit more context sensitive by
   looking at the possible parameter and see if it is plausible, which
   hopefully would work for most of the real life cases (e.g. "--abbrev
   7 HEAD" vs "--abbrev HEAD").  I however agree with Pierre that "DWIM
   works most of the time" is not good enough if there is no way to
   disambiguate in cases that fall outside.

^ permalink raw reply

* Re: [PATCH] git-svn: unlink internal index files after operations
From: Karl Hasselström @ 2007-12-13 20:27 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <20071213162734.GA18433@soma>

On 2007-12-13 08:27:34 -0800, Eric Wong wrote:

> Being git, we can generate these very quickly on the fly as needed,
> so there's no point in wasting space for these things for large
> projects.

Excellent!

For existing repos, I assume the correct thing to do is to just blast
all the existing index files manually?

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: [RFH] convert shortlog to use parse_options
From: Junio C Hamano @ 2007-12-13 19:49 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Jeff King, git
In-Reply-To: <20071213180701.GF1224@artemis.madism.org>

Pierre Habouzit <madcoder@debian.org> writes:

> The other issue is that when you had --abbrev=foo or --abbrev foo, the
> first one has to generate an error, whereas the second one should just
> say "foo" is not for me. The point being that the callback is not really
> aware of how the argument got assigned to it.

That obviously needs to be fixed ("this is the next token that could be
yours" vs "this must be yours"), if we were to go the route I suggested.

^ permalink raw reply

* [PATCH] Don't use the pager when running "git diff --check"
From: Wincent Colaiuta @ 2007-12-13 19:45 UTC (permalink / raw)
  To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197552751-53480-2-git-send-email-win@wincent.com>

In 89d07f75 "git diff" learnt to not run the pager if the user passes
the --exit-code switch. This commit does the same for the --check
switch for the same reason: we want the user to get the exit status
from "git diff", not the pager.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---

This really should have been part of my original patch,
([PATCH 1/5] "diff --check" should affect exit status"), but I only
just discovered this now.

 builtin-diff.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index 86d01a3..ed2d218 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -247,7 +247,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	/* If the user asked for our exit code then don't start a
 	 * pager or we would end up reporting its exit code instead.
 	 */
-	if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+	if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS) &&
+	    (!&rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF))
 		setup_pager();
 
 	/* Do we have --cached and not have a pending object, then
-- 
1.5.4.rc0.5.gd201-dirty

^ permalink raw reply related

* Re: [RFH] convert shortlog to use parse_options
From: Junio C Hamano @ 2007-12-13 19:31 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Jeff King, git
In-Reply-To: <20071213180347.GE1224@artemis.madism.org>

Pierre Habouzit <madcoder@debian.org> writes:

> I thought of that, but it's really convoluted and can definitely lead
> to very subtle issues. The number of git commands with optional
> arguments is quite low, mostly due to legacy, I don't expect _new_
> commands to take optional arguments.

I do not think we want to assume anything like that for a generic API.

> I don't really like the ambiguity
> it creates, and in some cases you just won't be able to disambiguate at
> all. Here it looks nice because --abbrev takes an integer argument, and
> it's likely that no branch nor reference names will be only made of
> digits. Though for commands taking an optional string[0] argument this is
> way more fishy.

I thought about ambiguity issues and I was only 70% sure about my
suggestion.  If you have an object, the initial unique part of whose
name consists only of decimal digits, you could get:

	git describe --abbrev 7 538538538
	git describe --abbrev 538538538 HEAD
	git describe --abbrev 538538538

and the last case needs to be disambiguated (either show HEAD with
abbreviation of 540 million digits, or show that object abbreviated to
the default length).  Because we are discussing a generic "optional
integer with default value" parser, let's not argue that only a small
integer between 0..40 makes sense to --abbrev flag in this context and
we should use that knowledge to further disambiguate [*1*].

You could say "If a flag takes an optional parameter and has a default,
an empty string means use the default", and disambiguate the lat example
in the above this way:

	git describe --abbrev 538538538
	git describe --abbrev '' 538538538

But it is not prettier than always requiring the optional parameter to
be sticked with the flag, as you suggest, like this;

	git describe --abbrev 538538538
	git describe --abbrev=538538538

So I am not entirely opposed to your version, nor I am claiming my
suggestion is better.  However, I just thought that "some parameters you
MUST stick to the flag" might create confusion to the end users, and I
wanted to see if others can come up with a less confusing alternative.
And the way I did so was to keep the discussion going by stirring the
pot a bit.

>   [0] OTOH I'm not sure there will ever be optional arguments that
>       aren't integers in git, but I may be wrong.

We could make HEAD as the default for "git branch --contains", if you
want an example.

[Footnote]

*1* We could introduce "optional integer with valid range with default",
and that would fit naturally into the scheme I outlined.  Even if the
next token parses as an integer, if it is out of range, you can say it
is not yours, and if it has to be yours, you can barf, saying "that's
out of range".

"The valid range" would be useful regardless of disambiguation. I wished
for "only valid is 1 or more" when I adjusted one of the commands to
parse_options().

^ permalink raw reply

* Re: [RFH] convert shortlog to use parse_options
From: Kristian Høgsberg @ 2007-12-13 18:47 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <1197570521.28742.0.camel@hinata.boston.redhat.com>

On Thu, 2007-12-13 at 13:28 -0500, Kristian Høgsberg wrote:
> On Thu, 2007-12-13 at 19:03 +0100, Pierre Habouzit wrote:
> > On Thu, Dec 13, 2007 at 05:40:23PM +0000, Junio C Hamano wrote:
> > > Pierre Habouzit <madcoder@debian.org> writes:
> > > 
> > > > In fact we have kind of the issue for every single optional argument out
> > > > there:
> > > >
> > > > $ git describe --abbrev HEAD
> > > > error: option `abbrev' expects a numerical value
> > > > [...]
> > > >
> > > >   *ouch*
> > > >
> > > > So I believe that with optional arguments we must change the way we do
> > > > things, and that we _must_ enforce the argument to be sticked in that
> > > > case.
> > > 
> > > I think "Must" is a bit too strong an expression.
> > > 
> > > 	git describe --abbrev 7 HEAD
> > >         git describe --abbrev HEAD
> > >         git describe --abbrev=HEAD
> > > 	git describe --abbrev=7 HEAD
> > > 	git describe --abbrev
> > > 
> > > The --abbrev parser in this case could be asked with this question: "You
> > > are on the command line.  There is a token after you.  Is it your
> > > parameter?".
> > 
> >   I thought of that, but it's really convoluted and can definitely lead
> > to very subtle issues. The number of git commands with optional
> > arguments is quite low, mostly due to legacy, I don't expect _new_
> > commands to take optional arguments. I don't really like the ambiguity
> > it creates, and in some cases you just won't be able to disambiguate at
> > all. Here it looks nice because --abbrev takes an integer argument, and
> > it's likely that no branch nor reference names will be only made of
> > digits. Though for commands taking an optional string[0] argument this is
> > way more fishy.
> 
> My 

Oops, sorry about that.  I just wanted to say we shouldn't jump through
all these hoops to make the option parser support every type of option
there ever was in the git command line ui.  A lot of these were probably
decided somewhat arbitrarily by whoever implemented the command.
Instead it's an opportunity to retroactively enforce some consistency
and predictability to the various option-styles that have been
hand-rolled over time in different git commands.

Kristian

^ 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