Git development
 help / color / mirror / Atom feed
* Re: [BUG] git-diff-* --color oddness
From: Jeff King @ 2008-01-04  8:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, win
In-Reply-To: <7vsl1ekmg5.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 04, 2008 at 12:46:18AM -0800, Junio C Hamano wrote:

> > It's for user diff drivers, and it looks like the funcname pattern. Not
> > sure why we want to demand-load that stuff at all...I wonder if it
> > should just go into git_default_config (or a git_basic_diff_config).
> 
> Yeah, moving some to the diff-basic-config sounds sane.

OK, below is a patch that fixes the color issues I was having.

>  * I'd prefer to keep low-level produce consistent diff that can
>    be reliably applied with git-apply without getting broken by
>    user configuration while Porcelain level diff can be tweaked
>    by the user to do whatever "human readable" crap they would
>    want to see on the screen (such as "side by side"), and my
>    gut feeling is that we should keep the user-level driver
>    definition in ui-config, never to affect plumbing.

OK. In that case, we need a way for the plumbing to tell the diff
machinery "don't ever try loading the ui config."

>  * using or not-using colors should stay in ui-config;

Agreed.

>  * funcname-pattern can go either way; that affects what appears
>    at the end of @@ context @@ lines, and would not have risk to
>    corrupt the patch for plumbing.

I don't personally use this, so I don't care too much. But I am inclined
to say low-level since it cannot break the patch (and it will be used
and presented by porcelains like "git add -i").

>  * color choice can also go either way, but probably is better
>    to be in the low-level.  Cnce color is used the output cannot
>    be fed sanely to patch or git-apply _anyway_ so we can be
>    sure that "git diff-plumbing --color" is run to produce human
>    readable output.

Definitely low-level IMHO, since we have porcelain which relies on the
low-level's output (and there is no way to set it manually from the
command line).

Patch to add diff_basic_config infrastructure and fix the color
selection issue is below. It just does the color, but we can move other
stuff over as discussion ensues.

-- >8 --
add a "basic" diff config callback

The diff porcelain uses git_diff_ui_config to set
porcelain-ish config options, like automatically turning on
color. The plumbing specifically avoids calling this
function, since it doesn't want things like automatic color
or rename detection.

However, some diff options should be set for both plumbing
and porcelain. For example, one can still turn on color in
git-diff-files using the --color command line option. This
means we want the color config from color.diff.* (so that
once color is on, we use the user's preferred scheme), but
_not_ the color.diff variable.

We split the diff config into "ui" and "basic", where
"basic" is suitable for use by plumbing (so _most_ things
affecting the output should still go into the "ui" part).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-diff-files.c |    2 +-
 builtin-diff-index.c |    2 +-
 builtin-diff-tree.c  |    2 +-
 diff.c               |    6 ++++++
 diff.h               |    1 +
 5 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 9c04111..4abe3c2 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -21,7 +21,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 
 	prefix = setup_git_directory_gently(&nongit);
 	init_revisions(&rev, prefix);
-	git_config(git_default_config); /* no "diff" UI options */
+	git_config(git_diff_basic_config); /* no "diff" UI options */
 	rev.abbrev = 0;
 
 	if (!setup_diff_no_index(&rev, argc, argv, nongit, prefix))
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 0f2390a..2b955de 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -17,7 +17,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 	int result;
 
 	init_revisions(&rev, prefix);
-	git_config(git_default_config); /* no "diff" UI options */
+	git_config(git_diff_basic_config); /* no "diff" UI options */
 	rev.abbrev = 0;
 
 	argc = setup_revisions(argc, argv, &rev, NULL);
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index ebc50ef..832797f 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -68,7 +68,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	int read_stdin = 0;
 
 	init_revisions(opt, prefix);
-	git_config(git_default_config); /* no "diff" UI options */
+	git_config(git_diff_basic_config); /* no "diff" UI options */
 	nr_sha1 = 0;
 	opt->abbrev = 0;
 	opt->diff = 1;
diff --git a/diff.c b/diff.c
index f6a8515..6bb0f62 100644
--- a/diff.c
+++ b/diff.c
@@ -183,6 +183,12 @@ int git_diff_ui_config(const char *var, const char *value)
 				return parse_funcname_pattern(var, ep, value);
 		}
 	}
+
+	return git_diff_basic_config(var, value);
+}
+
+int git_diff_basic_config(const char *var, const char *value)
+{
 	if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
 		int slot = parse_diff_color_slot(var, 11);
 		color_parse(value, var, diff_colors[slot]);
diff --git a/diff.h b/diff.h
index beccf85..073d5cb 100644
--- a/diff.h
+++ b/diff.h
@@ -172,6 +172,7 @@ extern void diff_unmerge(struct diff_options *,
 #define DIFF_SETUP_USE_CACHE		2
 #define DIFF_SETUP_USE_SIZE_CACHE	4
 
+extern int git_diff_basic_config(const char *var, const char *value);
 extern int git_diff_ui_config(const char *var, const char *value);
 extern void diff_setup(struct diff_options *);
 extern int diff_opt_parse(struct diff_options *, const char **, int);
-- 
1.5.4.rc2.1123.gce734-dirty

^ permalink raw reply related

* Re: [BUG] git-diff-* --color oddness
From: Junio C Hamano @ 2008-01-04  8:46 UTC (permalink / raw)
  To: Jeff King; +Cc: git, win
In-Reply-To: <20080104083252.GB3300@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Jan 04, 2008 at 12:26:55AM -0800, Junio C Hamano wrote:
>
>> > The first two lines of meta-info will be in the stock colors, but
>> > everything after will be in the custom colors. So we are actually
>> > reading the diff_ui options _during_ the diff. The culprit is
>> > funcname_pattern, which calls read_config_if_needed.
>> 
>> Yuck.  Why is funcname_pattern do ui-config stuff?  I know it
>> wants to get custom regexp crap, but that should belong to the
>> plumbing part, not Porcelain-only thing, shouldn't it?
>
> It's for user diff drivers, and it looks like the funcname pattern. Not
> sure why we want to demand-load that stuff at all...I wonder if it
> should just go into git_default_config (or a git_basic_diff_config).

Yeah, moving some to the diff-basic-config sounds sane.

 * I'd prefer to keep low-level produce consistent diff that can
   be reliably applied with git-apply without getting broken by
   user configuration while Porcelain level diff can be tweaked
   by the user to do whatever "human readable" crap they would
   want to see on the screen (such as "side by side"), and my
   gut feeling is that we should keep the user-level driver
   definition in ui-config, never to affect plumbing.

 * using or not-using colors should stay in ui-config;

 * funcname-pattern can go either way; that affects what appears
   at the end of @@ context @@ lines, and would not have risk to
   corrupt the patch for plumbing.

 * color choice can also go either way, but probably is better
   to be in the low-level.  Cnce color is used the output cannot
   be fed sanely to patch or git-apply _anyway_ so we can be
   sure that "git diff-plumbing --color" is run to produce human
   readable output.

^ permalink raw reply

* Re: [BUG] git-diff-* --color oddness
From: Jeff King @ 2008-01-04  8:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, win
In-Reply-To: <7v63yam1i9.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 04, 2008 at 12:35:42AM -0800, Junio C Hamano wrote:

> What I meant is "git diff" without --color can be colorized
> because of config but we should never allow "git diff-files"
> without --color to be colorized by user's config.  I realize
> that you were talking about the choice of colors, which is a
> different issue.
> 
> I do not much care ;-), but I guess we would want to be
> consistent.

OK, yes, I knew that about diff.color already. But I think it is a bug
to not use the user's colors in "git add -i", and I think the right fix
is to make diff-files consistent in its color choices. Patch will
follow.

> That one _is_ a bug.  diff-files should not be affected by
> "diff.color = auto" or somesuch in the config, even when the
> user uses custom function header crap.

It doesn't use it, actually, but I think that is a happy accident of the
way most config options are used (i.e., we _do_ read and change the
config, but we just never look at it again because we have done our
diff_setup). If you could provoke any of the diff plumbing to call
diff_setup twice, you would be in trouble (but even diff-tree --stdin
doesn't do that).

-Peff

^ permalink raw reply

* Re: [PATCH] diff: try lazy read of config only once
From: Jeff King @ 2008-01-04  8:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1w8ym1g6.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 04, 2008 at 12:36:57AM -0800, Junio C Hamano wrote:

> Thanks.  I think you can lose "if (!user_diff_tail)" there as
> well.

Oops, yes. Please mark it up when you apply.

-Peff

^ permalink raw reply

* Re: [PATCH] diff: try lazy read of config only once
From: Junio C Hamano @ 2008-01-04  8:36 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20080104083116.GA3354@coredump.intra.peff.net>

Thanks.  I think you can lose "if (!user_diff_tail)" there as
well.

^ permalink raw reply

* Re: [BUG] git-diff-* --color oddness
From: Junio C Hamano @ 2008-01-04  8:35 UTC (permalink / raw)
  To: Jeff King; +Cc: git, win
In-Reply-To: <20080104082842.GA3300@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I'm not sure what you mean here. Are you saying that it is the desired
> behavior for "git-diff --color" to use my color.diff.* variables, but
> for "git-diff-files --color" not to?

What I meant is "git diff" without --color can be colorized
because of config but we should never allow "git diff-files"
without --color to be colorized by user's config.  I realize
that you were talking about the choice of colors, which is a
different issue.

I do not much care ;-), but I guess we would want to be
consistent.

> Not to mention the other bug (that diff-files _does_ read the config,
> just halfway through).

That one _is_ a bug.  diff-files should not be affected by
"diff.color = auto" or somesuch in the config, even when the
user uses custom function header crap.

^ permalink raw reply

* [PATCH] add--interactive: allow diff colors without interactive colors
From: Jeff King @ 2008-01-04  8:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Users with color.diff set to true/auto will not see color in
"git add -i" unless they also set color.interactive.

However, some users may want just one without the other, so
there's no reason to tie them together.

Note that there is now no way to have color on for "git
diff" but not for diffs from "git add -i"; such a
configuration seems unlikely, though.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-add--interactive.perl |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 0cdd800..aaa9b24 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -22,16 +22,16 @@ if ($use_color) {
 	$header_color = $repo->get_color("color.interactive.header", "bold");
 	$help_color = $repo->get_color("color.interactive.help", "red bold");
 	$normal_color = $repo->get_color("", "reset");
+}
 
-	# Do we also set diff colors?
-	$diff_use_color = $repo->get_colorbool('color.diff');
-	if ($diff_use_color) {
-		$new_color = $repo->get_color("color.diff.new", "green");
-		$old_color = $repo->get_color("color.diff.old", "red");
-		$fraginfo_color = $repo->get_color("color.diff.frag", "cyan");
-		$metainfo_color = $repo->get_color("color.diff.meta", "bold");
-		$whitespace_color = $repo->get_color("color.diff.whitespace", "normal red");
-	}
+# Do we also set diff colors?
+$diff_use_color = $repo->get_colorbool('color.diff');
+if ($diff_use_color) {
+	$new_color = $repo->get_color("color.diff.new", "green");
+	$old_color = $repo->get_color("color.diff.old", "red");
+	$fraginfo_color = $repo->get_color("color.diff.frag", "cyan");
+	$metainfo_color = $repo->get_color("color.diff.meta", "bold");
+	$whitespace_color = $repo->get_color("color.diff.whitespace", "normal red");
 }
 
 sub colored {
-- 
1.5.4.rc2.1122.g6954-dirty

^ permalink raw reply related

* Re: [BUG] git-diff-* --color oddness
From: Jeff King @ 2008-01-04  8:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, win
In-Reply-To: <7vabnmm1ww.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 04, 2008 at 12:26:55AM -0800, Junio C Hamano wrote:

> > The first two lines of meta-info will be in the stock colors, but
> > everything after will be in the custom colors. So we are actually
> > reading the diff_ui options _during_ the diff. The culprit is
> > funcname_pattern, which calls read_config_if_needed.
> 
> Yuck.  Why is funcname_pattern do ui-config stuff?  I know it
> wants to get custom regexp crap, but that should belong to the
> plumbing part, not Porcelain-only thing, shouldn't it?

It's for user diff drivers, and it looks like the funcname pattern. Not
sure why we want to demand-load that stuff at all...I wonder if it
should just go into git_default_config (or a git_basic_diff_config).

-Peff

^ permalink raw reply

* [PATCH] diff: try lazy read of config only once
From: Jeff King @ 2008-01-04  8:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

The function read_config_if_needed used a list pointer to
determined whether it had already been called. However, if
the user had no diff drivers, the function would repeatedly
read the config (typically once per diffed file).

Instead, let's use a signal variable so that we read it only
once (note that we may actually still read it an extra time,
since the config may already have been read outside of this
function). This reduces the cost of

  git-whatchanged -p >/dev/null

from 34 seconds to 33 seconds.

Signed-off-by: Jeff King <peff@peff.net>
---
This are might be changed from the fallout of the other mail I just
sent, but I wanted to at least mention it. The speedup is not terribly
significant, but it just seems stupid to parse the config file thousands
of times. And I suspect on slow-I/O systems (like Cygwin) that it might
make more of a difference.

 diff.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index 5bdc111..f6a8515 100644
--- a/diff.c
+++ b/diff.c
@@ -61,10 +61,15 @@ static struct ll_diff_driver {
 
 static void read_config_if_needed(void)
 {
+	static int done = 0;
+	if (done)
+		return;
+
 	if (!user_diff_tail) {
 		user_diff_tail = &user_diff;
 		git_config(git_diff_ui_config);
 	}
+	done = 1;
 }
 
 /*
-- 
1.5.4.rc2.1122.g6954-dirty

^ permalink raw reply related

* Re: [BUG] git-diff-* --color oddness
From: Jeff King @ 2008-01-04  8:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, win
In-Reply-To: <7vejcym21q.fsf@gitster.siamese.dyndns.org>

On Fri, Jan 04, 2008 at 12:24:01AM -0800, Junio C Hamano wrote:

> > Ordinarily, I would say "nobody should use --color with diff-files". But
> > we do, in git-add--interactive (see Wincent's 4af756f3).
> 
> The plumbing deliberately not calling ui_config does not have
> anything to do with them supporting explicit --color options.

I'm not sure what you mean here. Are you saying that it is the desired
behavior for "git-diff --color" to use my color.diff.* variables, but
for "git-diff-files --color" not to?

I think that's insane, and it makes "git-add -i" depending on
git-diff-file's colorization wrong (since in other parts it uses the
color.diff.* variables).

Not to mention the other bug (that diff-files _does_ read the config,
just halfway through).

-Peff

^ permalink raw reply

* Re: [BUG] git-diff-* --color oddness
From: Junio C Hamano @ 2008-01-04  8:26 UTC (permalink / raw)
  To: Jeff King; +Cc: git, win
In-Reply-To: <20080104081429.GA30635@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> The first two lines of meta-info will be in the stock colors, but
> everything after will be in the custom colors. So we are actually
> reading the diff_ui options _during_ the diff. The culprit is
> funcname_pattern, which calls read_config_if_needed.

Yuck.  Why is funcname_pattern do ui-config stuff?  I know it
wants to get custom regexp crap, but that should belong to the
plumbing part, not Porcelain-only thing, shouldn't it?

^ permalink raw reply

* Re: [BUG] git-diff-* --color oddness
From: Junio C Hamano @ 2008-01-04  8:24 UTC (permalink / raw)
  To: Jeff King; +Cc: git, win
In-Reply-To: <20080104081429.GA30635@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I encountered some weirdness today with git-diff-{files,tree,index} and
> --color.  They parse the --color command line option, but do _not_ call
> git_config(git_diff_ui_config), so if the user has colors specified in
> the config, they are not used.

This is very deliberate.  ui_config is about the Porcelains.

> Ordinarily, I would say "nobody should use --color with diff-files". But
> we do, in git-add--interactive (see Wincent's 4af756f3).

The plumbing deliberately not calling ui_config does not have
anything to do with them supporting explicit --color options.

^ permalink raw reply

* [BUG] git-diff-* --color oddness
From: Jeff King @ 2008-01-04  8:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, win

I encountered some weirdness today with git-diff-{files,tree,index} and
--color.  They parse the --color command line option, but do _not_ call
git_config(git_diff_ui_config), so if the user has colors specified in
the config, they are not used.

Ordinarily, I would say "nobody should use --color with diff-files". But
we do, in git-add--interactive (see Wincent's 4af756f3). So we can
probably fix that with a "git_diff_minimal_ui_config" which does just
the colors, and use that from the plumbing diff scripts (or just move
the color stuff into git_default_config).

But it gets worse. Try this:

  # make a custom color
  git config color.diff.meta yellow
  # now show a plumbing diff
  git diff-tree --color -p 257f3020^ 257f3020

The first two lines of meta-info will be in the stock colors, but
everything after will be in the custom colors. So we are actually
reading the diff_ui options _during_ the diff. The culprit is
funcname_pattern, which calls read_config_if_needed.

It's my understanding that diff_ui_config is about porcelain diff
options, so it is a bit worrisome that in the middle of a plumbing diff
process, we load porcelain config. Fortunately, most of them aren't
looked at except in diff_setup, which has already run. Colors actually
affect globals which are used during the diff, and so are noticed.

So I think the best course is probably:
  1. before v1.5.4, we need to fix the color handling in "git-diff -i",
     either by declaring git-diff-files --color illegal and finding
     another way to do 4af756f3, or by moving the diff color
     functionality into the default config.

  2. Probably post-v1.5.4, some thought should be given to
     how the diff ui config is demand-loaded by
     diff.c:read_config_if_needed. I don't _think_ it is a problem for
     now except for the colors, because most parts just get used in
     diff_setup. But it seems a bit flaky.

-Peff

^ permalink raw reply

* Re: git-walkthrough-add script
From: Jeff King @ 2008-01-04  7:26 UTC (permalink / raw)
  To: William Morgan; +Cc: Git Mailing List
In-Reply-To: <1199426431-sup-6092@south>

On Thu, Jan 03, 2008 at 10:14:31PM -0800, William Morgan wrote:

> I've written a little script to do darcs-style hunk-by-hunk
> walkthroughs. It's based on the git-hunk-commit script that was floating
> around. Maybe someone else will find it useful.
> 
> http://git-wt-commit.rubyforge.org/

It didn't work for me, since the diff parsing failed to match my
particular colors (I needed /^.....diff/ instead of /^....diff/). I
suspect the color matching needs to be more flexible to be generally
useful.

However, I'm not clear what advantages this has over "git add -p".

-Peff

^ permalink raw reply

* Re: [ANNOUNCE] ugit: the pythonic git gui
From: Marco Costalba @ 2008-01-04  7:15 UTC (permalink / raw)
  To: David; +Cc: Guilhem Bonnefille, git
In-Reply-To: <402731c90801021920h284bb84dn29151dccd90eed2a@mail.gmail.com>

On Jan 3, 2008 4:20 AM, David <davvid@gmail.com> wrote:
>
> I don't have any fancy mac or windows installers at the moment (Linux
> is my primary platform) though I am working on packaging soon.
>

Regarding windows installer I was very happy with Inno Setuo that I
didn't know but I found out be very simple to learn and powerful.

It's a script that you tweak according to your needs, then run the
inno setup compiler on it and that's all. In case you need a script
example just as a starting point you could use the one I have created
for qgit, you can download directly from

http://git.kernel.org/?p=qgit/qgit4.git;a=tree

The script is called qgit_inno_setup.iss


Marco

^ permalink raw reply

* git-walkthrough-add script
From: William Morgan @ 2008-01-04  6:14 UTC (permalink / raw)
  To: Git Mailing List

Hi all,

I've written a little script to do darcs-style hunk-by-hunk
walkthroughs. It's based on the git-hunk-commit script that was floating
around. Maybe someone else will find it useful.

http://git-wt-commit.rubyforge.org/

-- 
William <wmorgan-git@masanjin.net>

^ permalink raw reply

* Re: [PATCH] Documentation: rename gitlink macro to linkgit
From: Yannick Gingras @ 2008-01-04  6:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: asciidoc-discuss, git
In-Reply-To: <7vir2am95w.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>> But the test suite can only catch obvious rendering failures so any
>> help in eyeballing the output will be appreciated.
>
> You could go fancy like that, but I suspect that an automated
> test to compare the text dump (e.g. "links -dump doc.html")
> generated by before and after version, perhaps with minimum
> massaging, would go a long enough way.  At least that would have
> caught the "gitlink" breakage, wouldn't it?

Yeah it would have caught it.  I'll hack something like that before
the 9.0 release.

-- 
Yannick Gingras

^ permalink raw reply

* Re: [PATCH] Documentation: rename gitlink macro to linkgit
From: Junio C Hamano @ 2008-01-04  5:50 UTC (permalink / raw)
  To: Yannick Gingras; +Cc: asciidoc-discuss, git
In-Reply-To: <873ateyxjk.fsf@enceladus.ygingras.net>

Yannick Gingras <ygingras@ygingras.net> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Stuart, is there anything we can help you to set up some automated
>> tests to catch AsciiDoc regression, so we do not have to suffer like
>> this again?
>
> We considered adding a nose test suite.  The upcoming v9.0 release
> involves quite a bit of code massaging and we will definitely need an
> extensive test suite.  But the test suite can only catch obvious
> rendering failures so any help in eyeballing the output will be
> appreciated.

You could go fancy like that, but I suspect that an automated
test to compare the text dump (e.g. "links -dump doc.html")
generated by before and after version, perhaps with minimum
massaging, would go a long enough way.  At least that would have
caught the "gitlink" breakage, wouldn't it?

^ permalink raw reply

* Re: [PATCH] Documentation: rename gitlink macro to linkgit
From: Yannick Gingras @ 2008-01-04  5:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: asciidoc-discuss, git
In-Reply-To: <7vejcypqsp.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Stuart, is there anything we can help you to set up some automated
> tests to catch AsciiDoc regression, so we do not have to suffer like
> this again?

We considered adding a nose test suite.  The upcoming v9.0 release
involves quite a bit of code massaging and we will definitely need an
extensive test suite.  But the test suite can only catch obvious
rendering failures so any help in eyeballing the output will be
appreciated.

Should we setup a distributed proof reading system like Project
Gutenberg?  I hope we don't need it.  But a few scripts to batch
render all the doc of popular projects would be nice.  We could upload
such snapshot render jobs where everyone could take a quick glance and
spot obvious rendering errors.

If you follow AsciiDoc's mailing list [1], we'll go through a series
of release candidates before the final 9.0.  "With enough eyeballs,
all bugs are shallow."

[1]: asciidoc-discuss@lists.metaperl.com

I reply to two mailing lists; I think this particular problem belongs
to asciidoc-discuss.

Best regards, 

-- 
Yannick Gingras

^ permalink raw reply

* [PATCH] git-clean: make "Would remove ..." path relative to cwd again
From: Junio C Hamano @ 2008-01-04  4:47 UTC (permalink / raw)
  To: git; +Cc: Miklos Vajna

The rewrite changed the output to use the path relative to the
top of the work tree without a good reason.  This fixes it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-clean.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/builtin-clean.c b/builtin-clean.c
index ae30d4e..6cad8ea 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c
@@ -34,6 +34,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	struct dir_struct dir;
 	const char *path, *base;
 	static const char **pathspec;
+	int prefix_offset = 0;
 	char *seen = NULL;
 	struct option options[] = {
 		OPT__QUIET(&quiet),
@@ -71,6 +72,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	if (!ignored)
 		setup_standard_excludes(&dir);
 
+	if (prefix)
+		prefix_offset = strlen(prefix);
 	pathspec = get_pathspec(prefix, argv);
 	read_cache();
 
@@ -132,26 +135,32 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		if (S_ISDIR(st.st_mode)) {
 			strbuf_addstr(&directory, ent->name);
 			if (show_only && (remove_directories || matches)) {
-				printf("Would remove %s\n", directory.buf);
+				printf("Would remove %s\n",
+				       directory.buf + prefix_offset);
 			} else if (quiet && (remove_directories || matches)) {
 				remove_dir_recursively(&directory, 0);
 			} else if (remove_directories || matches) {
-				printf("Removing %s\n", directory.buf);
+				printf("Removing %s\n",
+				       directory.buf + prefix_offset);
 				remove_dir_recursively(&directory, 0);
 			} else if (show_only) {
-				printf("Would not remove %s\n", directory.buf);
+				printf("Would not remove %s\n",
+				       directory.buf + prefix_offset);
 			} else {
-				printf("Not removing %s\n", directory.buf);
+				printf("Not removing %s\n",
+				       directory.buf + prefix_offset);
 			}
 			strbuf_reset(&directory);
 		} else {
 			if (pathspec && !matches)
 				continue;
 			if (show_only) {
-				printf("Would remove %s\n", ent->name);
+				printf("Would remove %s\n",
+				       ent->name + prefix_offset);
 				continue;
 			} else if (!quiet) {
-				printf("Removing %s\n", ent->name);
+				printf("Removing %s\n",
+				       ent->name + prefix_offset);
 			}
 			unlink(ent->name);
 		}
-- 
1.5.4.rc2.17.g257f

^ permalink raw reply related

* Re: wherefore art thou, git-applymbox? - Adding non-self signoffs
From: Junio C Hamano @ 2008-01-04  3:32 UTC (permalink / raw)
  To: Joel Becker; +Cc: git
In-Reply-To: <20080104015028.GC3989@mail.oracle.com>

Joel Becker <Joel.Becker@oracle.com> writes:

> We used to do this very easily with git-applymbox:
> (from http://oss.oracle.com/osswiki/GitRepositories/ForMaintainers)
>
> $ echo "Julie Hacker <julieh@my.site.com>" > /tmp/signoff
> $ git branch to-push master
> $ git checkout to-push
> $ git format-patch -C -k --stdout master..workingbranch > /tmp/changes-to-push
> $ git applymbox -k /tmp/changes-to-push /tmp/signoff
> $ git push ssh://my.server.com/path/project.git to-push:master
>
> 	The <signoff> file argument to applymbox allowed us to add the
> approvers signoff to an entire series in one go.  git-am does not have
> this feature.  As far as I can tell, I have to edit each patch by hand
> to add the new signoff.  Is there a better way?

Heh, applymbox's removal is an ancient news.  May 20 2007?

My reading of an old copy of git-applypatch seems to suggest
that the above example you quoted is probably wrong anyway;
shouldn't the first one be like this instead?

 $ echo "Signed-off-by: Julie Hacker <julieh@my.site.com>" >/tmp/signoff

It was actually a bug that applymbox allowed only a single
e-mail address to be added without doing any sanity checking of
the address with the author nor committer information.

If it were designed to allow adding sign-offs from other people,
the command would have allowed more than one lines in the file.
It did not.  It was not designed for that purpose.

It was designed to allow one's own sign-off; it should have
verified that it matched the committer identity.  It did not.
That was not strictly a bug for people who used the mechanism to
sign their own patches anyway, but not checking meant a misuse
like yours went unnoticed.  Not quite ideal.

I guess you can run filter-branch to munge the commit messages
after you run the

	git format-patch ... | git am

pipeline to build the to-push branch.

I do not mind a patch to enhance "git am", but not before 1.5.4.
Most likely the change would take a form of an extra parameter
that names a script (or command) that gets the commit log
message as its argument and edits it in any way it wants (in
your case you would add the S-o-b: lines in that script).

^ permalink raw reply

* [EGIT PATCH] Remove directory entries from Structure Compare view.
From: Roger C. Soares @ 2008-01-04  2:11 UTC (permalink / raw)
  To: git

Date: Wed, 2 Jan 2008 22:51:33 -0200

Signed-off-by: Roger C. Soares <rogersoares@intelinet.com.br>
---
Please evaluate.

 .../GitCompareFileRevisionEditorInput.java         |   25 +++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/GitCompareFileRevisionEditorInput.java 
b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/GitCompareFileRevisionEditorInput.java
index 82cd205..eaba1fa 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/GitCompareFileRevisionEditorInput.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/GitCompareFileRevisionEditorInput.java
@@ -21,6 +21,7 @@ import org.eclipse.compare.ITypedElement;
 import org.eclipse.compare.structuremergeviewer.DiffNode;
 import org.eclipse.compare.structuremergeviewer.Differencer;
 import org.eclipse.compare.structuremergeviewer.ICompareInput;
+import org.eclipse.compare.structuremergeviewer.IDiffElement;
 import org.eclipse.compare.structuremergeviewer.IStructureComparator;
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IFileState;
@@ -299,7 +300,29 @@ public class GitCompareFileRevisionEditorInput extends CompareEditorInput {
 		ensureContentsCached(getLeftRevision(), getRightRevision(), monitor);
 		initLabels(input);
 		setTitle(NLS.bind(TeamUIMessages.SyncInfoCompareInput_title, new String[] { input.getName() }));
-		return input;
+
+		// The compare editor (Structure Compare) will show the diff filenames
+		// with their project relative path. So, no need to also show directory entries.
+		DiffNode flatDiffNode = new DiffNode(null,Differencer.CHANGE,null,left,right);
+		flatDiffView(flatDiffNode, (DiffNode) input);
+
+		return flatDiffNode;
+	}
+
+	private void flatDiffView(DiffNode rootNode, DiffNode currentNode) {
+		if(currentNode != null) {
+			IDiffElement[] dElems = currentNode.getChildren();
+			if(dElems != null) {
+				for(IDiffElement dElem : dElems) {
+					DiffNode dNode = (DiffNode) dElem;
+					if(dNode.getChildren() != null && dNode.getChildren().length > 0) {
+						flatDiffView(rootNode, dNode);
+					} else {
+						rootNode.add(dNode);
+					}
+				}
+			}
+		}
 	}
 
 }
-- 
1.5.3.7

^ permalink raw reply related

* wherefore art thou, git-applymbox? - Adding non-self signoffs
From: Joel Becker @ 2008-01-04  1:50 UTC (permalink / raw)
  To: git

Junio, et al,
	When git-applymbox disappeared, I didn't pay much attention.  I
just learned git-am and went along.  Little did I know, there was a
trap laid.
	The ocfs2-tools.git repository is maintained by the entire ocfs2
team.  It's a "shared" style repo.  A proposed change is posted to
ocfs2-tools-devel, and when a teammate approves, they respond with a
signoff.  The author then adds the signoff to the patch and pushes to
the shared repo.
	We used to do this very easily with git-applymbox:
(from http://oss.oracle.com/osswiki/GitRepositories/ForMaintainers)

$ echo "Julie Hacker <julieh@my.site.com>" > /tmp/signoff
$ git branch to-push master
$ git checkout to-push
$ git format-patch -C -k --stdout master..workingbranch > /tmp/changes-to-push
$ git applymbox -k /tmp/changes-to-push /tmp/signoff
$ git push ssh://my.server.com/path/project.git to-push:master

	The <signoff> file argument to applymbox allowed us to add the
approvers signoff to an entire series in one go.  git-am does not have
this feature.  As far as I can tell, I have to edit each patch by hand
to add the new signoff.  Is there a better way?

Joel

-- 

Life's Little Instruction Book #356

	"Be there when people need you."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

^ permalink raw reply

* Re: git over webdav: what can I do for improving http-push ?
From: Martin Langhoff @ 2008-01-03 23:54 UTC (permalink / raw)
  To: Jan Hudec; +Cc: Grégoire Barbier, Jakub Narebski, git
In-Reply-To: <20080103211521.GA4225@efreet.light.src>

On Jan 4, 2008 10:15 AM, Jan Hudec <bulb@ucw.cz> wrote:
> Now to keep it stateless, I thought that:
...
> This would guarantee, that when you want n revisions, you make at most
> log2(n) requests and get at most 2*n revisions (well, the requests are for

That is still a lot! How about, for each ref

 - Client sends a POST listing the ref and the latest related commit
it has that the server is likely to have (from origin/heads/<ref>).
Optionally, it can provide a blacklist of <treeish> (where every
object refered is known) and blob sha1s.
 - Server sends the new sha1 of the ref, and a thin pack that covers the changes
 - The client can disconnect to stop the transaction. For example --
if it sees the sha1 of a huge object that it already has. It can
re-request, with a blacklist.

A good number of objects will be sent unnecesarily - with no option to
the client to say "I have this" - but by using the hint of letting the
server know we have origin/heads/<ref> I suspect that it will be
minimal.

Also:
 - It will probably be useful to list all the refs the client knows
from that server in the request.
 - If the ref has changed with a non-fast-forward, the server needs to
say so, and provide a listing of the commits. As soon as the client
spots a common commit, it can close the connection -- it now knows
what ref to tell the server about in a subsequent command.

This way, you ideally have 1 request per ref, 2 if it has been
rebased/rewound. This can probably get reorganised to do several refs
in one request.

cheers,


m

^ permalink raw reply

* Re: git over webdav: what can I do for improving http-push ?
From: Grégoire Barbier @ 2008-01-03 23:29 UTC (permalink / raw)
  To: git
In-Reply-To: <200801032247.02323.jnareb@gmail.com>

Jakub Narebski a écrit :
> Perhaps we could use AJAX (XMLHttpRequest for communication, plain HTTP or
> IFRAMES for sending data) or something like this for git protocol tunneling?
>   
well... I think I may manage to avoid javascript... ;-)
more seriously, I was thinking of using http in an automated, 
un-human-browsable manner, not a full html user interface.

-- 
Grégoire Barbier - gb à gbarbier.org - +33 6 21 35 73 49

^ 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