Git development
 help / color / mirror / Atom feed
* Re: [PATCH v4] user-manual: Add section "Why bisecting merge commits can be harder ..."
From: Steffen Prohaska @ 2007-11-10 22:41 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Linus Torvalds, Benoit Sigoure, Andreas Ericsson, Johannes Sixt,
	git
In-Reply-To: <7vtzntlufh.fsf@gitster.siamese.dyndns.org>


On Nov 10, 2007, at 9:25 PM, Junio C Hamano wrote:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> On Sat, 10 Nov 2007, Steffen Prohaska wrote:
>>> +
>>> +But if you already made a merge C instead of rebasing, all
>>> +is not lost.  In the illustrated case, you can easily rebase
>>> +one parent branch on top of the other after the fact, just
>>> +to understand the history and to make the history more
>>> +easily to bisect.
>>
>> I simply don't think this is true.
>>
>> You can *not* easily rebase after the fact. Both the parents may  
>> have lots
>> of merges independently of each other, and rebase won't be able to do
>> *anything* with such a case. In fact, the only reason you think  
>> you can
>> easily rebase after-the-fact is that you made the example history be
>> trivial. In real life, if the example history is that trivial (just a
>> single merge of two otherwise linear histories), you'd probably  
>> generally
>> not have this problem in the first place.
>>
>> The facts are:
>>
>>  - git bisect can handle merges quite well, and points to the right
>>    commit (which is *usually* not a merge).
>>
>>  - but merges by definition introduce changes from two independent  
>> lines
>>    of development, and while both lines may work well on their  
>> own, it is
>>    possible that (a) the merge itself was done incorrectly or (b)  
>> the two
>>    (or more) features that were introduced simply clash.
>>
>>  - "git rebase" won't do a thing for this after the fact, except in
>>    trivial cases, and even then it will generate a new history  
>> that simply
>>    isn't compatible with the original history, so while it could  
>> in theory
>>    help bisect things further in some limited and simple cases, in  
>> general
>>    it's simply not that useful an approach.
>>
>> ..and I think it's simply wrong to even *try* to imply that "git  
>> rebase"
>> can somehow change any of this.
>
> Very true.  It is a suggestion useful _only_ when you can easily
> rebase.  Like the one illustrated in the description, which is
> artificial and of very limited scope.  If you cannot rebase
> easily, then (by definition) rebase would not help you.

I propose to remove this suggestion if no one comes up with a
real world example where rebasing after the fact turned out to
be useful. If nobody has such an example it doesn't make sense
to tell readers of the manual that rebase could be useful in
such a situation.

The patch would be identical except for the last paragraph
removed.

	Steffen

^ permalink raw reply

* Re: [PATCH] Make git-clean a builtin
From: Miles Bader @ 2007-11-10 22:43 UTC (permalink / raw)
  To: Bill Lear; +Cc: Johannes Schindelin, Shawn Bohrer, gitster, git
In-Reply-To: <18225.48553.44088.269677@lisa.zopyra.com>

Bill Lear <rael@zopyra.com> writes:
> Why do you find it objectionable?

It bloats the code, and makes it less readable.

[My conjecture is that the latter happens because braces are so visually
striking that they attract the eye; for _long_ blocks, this property of
braces helps, because it makes it easier to find them amongst the rest
of the code, but for _short_ blocks, it hurts, because it draws the eye
away from the actual code, and emphasizes structure at a time when
structure doesn't need emphasizing (because it's utterly obvious already
with such short blocks).]

-Miles

-- 
Run away!  Run away!

^ permalink raw reply

* Re: t7005 and vi in GIT_EXEC_PATH
From: Brian Gernhardt @ 2007-11-10 22:45 UTC (permalink / raw)
  To: David Kastrup; +Cc: Git Mailing List
In-Reply-To: <85abpl69ck.fsf@lola.goethe.zz>


On Nov 10, 2007, at 5:09 PM, David Kastrup wrote:

> Brian Gernhardt <benji@silverinsanity.com> writes:
>
>> If vi is in GIT_EXEC_PATH, then t7005-editor.sh fails because the  
>> real
>> vi is invoked instead of the test vi script.  This is because the git
>> wrapper puts GIT_EXEC_PATH ahead of ".".  I see no easy solution to
>> this problem, and thought I should bring it up with the list.
>
> Putting "." at the front of GIT_EXEC_PATH instead of PATH would appear
> to do the trick then, wouldn't it?

The GIT_EXEC_PATH I was referring to is the one set in the Makefile  
and compiled into git.  The GIT_EXEC_PATH environment variable is set  
to the git repository.  PATH ends up looking like this (paraphrased):   
"git.git:install directory:.:normal PATH".  And since the install  
directory contains vi, the test fails (actually appears to hang  
because vi is waiting for input while it's output is being sent to / 
dev/null).

~~ Brian

^ permalink raw reply

* Re: Inconsistencies with git log
From: Miles Bader @ 2007-11-10 22:51 UTC (permalink / raw)
  To: Jon Smirl; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <9e4733910711071503va92a653s25fd978989d5917d@mail.gmail.com>

"Jon Smirl" <jonsmirl@gmail.com> writes:
> But if you type 'git log' while cd'd into a subdirectory the whole log
> is almost never what you want. It's this kind of thing that makes git
> harder to use.

Actually I almost always want the "whole project history" when cd'd to a
subdir.

Git's convention of doing the whole project by default, but allowing "."
(or any other directory name) to narrow it down seems almost perfect to
me.

-Miles

-- 
"Don't just question authority,
Don't forget to question me."
-- Jello Biafra

^ permalink raw reply

* Deprecate git-fetch-pack?
From: Daniel Barkalow @ 2007-11-10 23:11 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Now that git-fetch is in C, built in, and doing the fetch-pack in the same 
process, the normal usage patterns don't involve actually executing 
git-fetch-pack. Can we deprecate it at this point, or it is plausibly 
being used by scripts? As it is now, I'm not entirely confidant that the 
tests in t5500 won't be fooled by git-fetch working even with 
git-fetch-pack being broken in various ways, which should be fixed if we 
want to keep it.

We also might as well deprecate peek-remote now that it's a synonym for 
ls-remote.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH] Reduce the number of connects when fetching
From: Daniel Barkalow @ 2007-11-10 23:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This shares the connection between getting the remote ref list and
getting objects in the first batch. (A second connection is still used
to follow tags)
---
As far as I can tell, this works and is suitable for pu. But it may well 
have subtle issues, and may nearly totally break git-fetch-pack (as an 
independant program). FWIW, I didn't get any feedback other than general 
encouragement last time I tried.

 builtin-fetch-pack.c |   74 ++++++++++++++++++++++++++-----------------------
 fetch-pack.h         |    2 +
 transport.c          |   41 +++++++++++++++++++--------
 3 files changed, 70 insertions(+), 47 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index bb1742f..688a8dc 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -7,6 +7,7 @@
 #include "pack.h"
 #include "sideband.h"
 #include "fetch-pack.h"
+#include "remote.h"
 #include "run-command.h"
 
 static int transfer_unpack_limit = -1;
@@ -558,14 +559,14 @@ static int get_pack(int xd[2], char **pack_lockfile)
 }
 
 static struct ref *do_fetch_pack(int fd[2],
+		const struct ref *orig_ref,
 		int nr_match,
 		char **match,
 		char **pack_lockfile)
 {
-	struct ref *ref;
+	struct ref *ref = copy_ref_list(orig_ref);
 	unsigned char sha1[20];
 
-	get_remote_heads(fd[0], &ref, 0, NULL, 0);
 	if (is_repository_shallow() && !server_supports("shallow"))
 		die("Server does not support shallow clients");
 	if (server_supports("multi_ack")) {
@@ -583,10 +584,6 @@ static struct ref *do_fetch_pack(int fd[2],
 			fprintf(stderr, "Server supports side-band\n");
 		use_sideband = 1;
 	}
-	if (!ref) {
-		packet_flush(fd[1]);
-		die("no matching remote head");
-	}
 	if (everything_local(&ref, nr_match, match)) {
 		packet_flush(fd[1]);
 		goto all_done;
@@ -660,7 +657,7 @@ static void fetch_pack_setup(void)
 int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 {
 	int i, ret, nr_heads;
-	struct ref *ref;
+	struct ref *ref = NULL;
 	char *dest = NULL, **heads;
 
 	nr_heads = 0;
@@ -716,9 +713,34 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	if (!dest)
 		usage(fetch_pack_usage);
 
-	ref = fetch_pack(&args, dest, nr_heads, heads, NULL);
+	int fd[2];
+	struct child_process *conn = git_connect(fd, (char *)dest, args.uploadpack,
+                          args.verbose ? CONNECT_VERBOSE : 0);
+	if (conn) {
+		get_remote_heads(fd[0], &ref, 0, NULL, 0);
+
+		ref = fetch_pack(&args, fd, conn, ref, dest, nr_heads, heads, NULL);
+		close(fd[0]);
+		close(fd[1]);
+		if (finish_connect(conn))
+			ref = NULL;
+	} else {
+		ref = NULL;
+	}
 	ret = !ref;
 
+	if (!ret && nr_heads) {
+		/* If the heads to pull were given, we should have
+		 * consumed all of them by matching the remote.
+		 * Otherwise, 'git-fetch remote no-such-ref' would
+		 * silently succeed without issuing an error.
+		 */
+		for (i = 0; i < nr_heads; i++)
+			if (heads[i] && heads[i][0]) {
+				error("no such remote ref %s", heads[i]);
+				ret = 1;
+			}
+	}
 	while (ref) {
 		printf("%s %s\n",
 		       sha1_to_hex(ref->old_sha1), ref->name);
@@ -729,16 +751,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 }
 
 struct ref *fetch_pack(struct fetch_pack_args *my_args,
+		       int fd[], pid_t pid,
+		       const struct ref *ref,
 		const char *dest,
 		int nr_heads,
 		char **heads,
 		char **pack_lockfile)
 {
-	int i, ret;
-	int fd[2];
-	struct child_process *conn;
-	struct ref *ref;
 	struct stat st;
+	struct ref *ref_cpy;
 
 	fetch_pack_setup();
 	memcpy(&args, my_args, sizeof(args));
@@ -747,29 +768,15 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 			st.st_mtime = 0;
 	}
 
-	conn = git_connect(fd, (char *)dest, args.uploadpack,
-                          args.verbose ? CONNECT_VERBOSE : 0);
 	if (heads && nr_heads)
 		nr_heads = remove_duplicates(nr_heads, heads);
-	ref = do_fetch_pack(fd, nr_heads, heads, pack_lockfile);
-	close(fd[0]);
-	close(fd[1]);
-	ret = finish_connect(conn);
-
-	if (!ret && nr_heads) {
-		/* If the heads to pull were given, we should have
-		 * consumed all of them by matching the remote.
-		 * Otherwise, 'git-fetch remote no-such-ref' would
-		 * silently succeed without issuing an error.
-		 */
-		for (i = 0; i < nr_heads; i++)
-			if (heads[i] && heads[i][0]) {
-				error("no such remote ref %s", heads[i]);
-				ret = 1;
-			}
+	if (!ref) {
+		packet_flush(fd[1]);
+		die("no matching remote head");
 	}
+	ref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);
 
-	if (!ret && args.depth > 0) {
+	if (args.depth > 0) {
 		struct cache_time mtime;
 		char *shallow = git_path("shallow");
 		int fd;
@@ -798,8 +805,5 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 		}
 	}
 
-	if (ret)
-		ref = NULL;
-
-	return ref;
+	return ref_cpy;
 }
diff --git a/fetch-pack.h b/fetch-pack.h
index a7888ea..8d35ef6 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -16,6 +16,8 @@ struct fetch_pack_args
 };
 
 struct ref *fetch_pack(struct fetch_pack_args *args,
+		int fd[], struct child_process *conn,
+		const struct ref *ref,
 		const char *dest,
 		int nr_heads,
 		char **heads,
diff --git a/transport.c b/transport.c
index 83677fc..3cb0115 100644
--- a/transport.c
+++ b/transport.c
@@ -568,6 +568,8 @@ struct git_transport_data {
 	unsigned thin : 1;
 	unsigned keep : 1;
 	int depth;
+	struct child_process *conn;
+	int fd[2];
 	const char *uploadpack;
 	const char *receivepack;
 };
@@ -598,20 +600,20 @@ static int set_git_option(struct transport *connection,
 	return 1;
 }
 
+static int connect_setup(struct transport *transport)
+{
+	struct git_transport_data *data = transport->data;
+	data->conn = git_connect(data->fd, transport->url, data->uploadpack, 0);
+	return 0;
+}
+
 static struct ref *get_refs_via_connect(struct transport *transport)
 {
 	struct git_transport_data *data = transport->data;
 	struct ref *refs;
-	int fd[2];
-	char *dest = xstrdup(transport->url);
-	struct child_process *conn = git_connect(fd, dest, data->uploadpack, 0);
 
-	get_remote_heads(fd[0], &refs, 0, NULL, 0);
-	packet_flush(fd[1]);
-
-	finish_connect(conn);
-
-	free(dest);
+	connect_setup(transport);
+	get_remote_heads(data->fd[0], &refs, 0, NULL, 0);
 
 	return refs;
 }
@@ -622,7 +624,7 @@ static int fetch_refs_via_pack(struct transport *transport,
 	struct git_transport_data *data = transport->data;
 	char **heads = xmalloc(nr_heads * sizeof(*heads));
 	char **origh = xmalloc(nr_heads * sizeof(*origh));
-	struct ref *refs;
+	const struct ref *refs;
 	char *dest = xstrdup(transport->url);
 	struct fetch_pack_args args;
 	int i;
@@ -637,13 +639,27 @@ static int fetch_refs_via_pack(struct transport *transport,
 
 	for (i = 0; i < nr_heads; i++)
 		origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
-	refs = fetch_pack(&args, dest, nr_heads, heads, &transport->pack_lockfile);
+
+	refs = transport_get_remote_refs(transport);
+	if (!data->conn) {
+		struct ref *refs_tmp;
+		connect_setup(transport);
+		get_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0);
+		free_refs(refs_tmp);
+	}
+
+	refs = fetch_pack(&args, data->fd, data->conn, transport->remote_refs,
+			  dest, nr_heads, heads, &transport->pack_lockfile);
+	close(data->fd[0]);
+	close(data->fd[1]);
+	if (finish_connect(data->conn))
+		refs = NULL;
+	data->conn = NULL;
 
 	for (i = 0; i < nr_heads; i++)
 		free(origh[i]);
 	free(origh);
 	free(heads);
-	free_refs(refs);
 	free(dest);
 	return 0;
 }
@@ -725,6 +741,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		ret->disconnect = disconnect_git;
 
 		data->thin = 1;
+		data->conn = NULL;
 		data->uploadpack = "git-upload-pack";
 		if (remote && remote->uploadpack)
 			data->uploadpack = remote->uploadpack;
-- 
1.5.3.5.628.g8e762-dirty

^ permalink raw reply related

* Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
From: Lars Hjemli @ 2007-11-11  0:07 UTC (permalink / raw)
  To: Ping Yin; +Cc: git
In-Reply-To: <1194722863-14741-1-git-send-email-pkufranky@gmail.com>

On Nov 10, 2007 8:27 PM, Ping Yin <pkufranky@gmail.com> wrote:
> This commit teaches git status/commit to also show commits of user-cared
> modified submodules since HEAD (or HEAD^ if --amend option is on).

Some nitpicks:
-we'll need a config option to enable/disable this output in git-status
-the feature should probably be implemented in git-submodule.sh

--
larsh

^ permalink raw reply

* Re: Deprecate git-fetch-pack?
From: Junio C Hamano @ 2007-11-11  0:48 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711101752490.29952@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> Now that git-fetch is in C, built in, and doing the fetch-pack in the same 
> process, the normal usage patterns don't involve actually executing 
> git-fetch-pack. Can we deprecate it at this point, or it is plausibly 
> being used by scripts? As it is now, I'm not entirely confidant that the 
> tests in t5500 won't be fooled by git-fetch working even with 
> git-fetch-pack being broken in various ways, which should be fixed if we 
> want to keep it.
>
> We also might as well deprecate peek-remote now that it's a synonym for 
> ls-remote.

Especially because git-fetch is no longer as hackable as it used
to be, and because people may still find special needs that can
be hacked up with direct access to low level transports from the
script more easily than going down to the C level, I'd rather
wait and see for a cycle or two to decide.  There is no strong
reason to drop it, is there?

As to peek-remote, ls-remote over the native transport is a
synonym so I do not think anybody doing the scripting would have
problem doing s/peek-/ls-/ to their script, but again I do not
see a heavy maintenance burden to keep the synonym, yet.

^ permalink raw reply

* [PATCH 3/3] Added diff hunk coloring to git-add--interactive
From: Dan Zwell @ 2007-11-11  0:03 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Shawn O. Pearce, Wincent Colaiuta,
	Git Mailing List, Jonathan del Strother, Johannes Schindelin,
	Frank Lichtenheld
In-Reply-To: <20071104054305.GA13929@sigill.intra.peff.net>

Added and integrated method "color_diff_hunk", which colors
lines, and returns them in an array. Coloring bad whitespace is
not yet supported.

Signed-off-by: Dan Zwell <dzwell@zwell.net>
---
 git-add--interactive.perl |   58 ++++++++++++++++++++++++++++++++++++++------
 1 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 508531f..d92e8ed 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -3,7 +3,11 @@
 use strict;
 use Git;
 
+# Prompt colors:
 my ($use_color, $prompt_color, $header_color, $help_color, $normal_color);
+# Diff colors:
+my ($diff_use_color, $new_color, $old_color, $fraginfo_color,
+    $metainfo_color, $whitespace_color);
 my $color_config = qx(git config --get color.interactive);
 if ($color_config=~/true|always/ || -t STDOUT && $color_config=~/auto/) {
 	eval { require Term::ANSIColor; };
@@ -21,6 +25,24 @@ if ($color_config=~/true|always/ || -t STDOUT && $color_config=~/auto/) {
 		$help_color = Git::color_to_ansi_code(
 			Git::config($repo, "color.interactive.help") || "red bold");
 		$normal_color = Git::color_to_ansi_code("normal");
+
+		# Do we also set diff colors?
+		my $diff_colors = Git::config($repo, "color.diff");
+		if ($diff_colors=~/true/ ||
+			-t STDOUT && $diff_colors=~/auto/) {
+			$diff_use_color = 1;
+			$new_color = Git::color_to_ansi_code(
+				Git::config($repo, "color.diff.new") || "green");
+			$old_color = Git::color_to_ansi_code(
+				Git::config($repo, "color.diff.old") || "red");
+			$fraginfo_color = Git::color_to_ansi_code(
+				Git::config($repo, "color.diff.frag") || "cyan");
+			$metainfo_color = Git::color_to_ansi_code(
+				Git::config($repo, "color.diff.meta") || "bold");
+			# Not implemented:
+			#$whitespace_color = Git::color_to_ansi_code(
+				#Git::config($repo, "color.diff.whitespace") || "normal red");
+		}
 	}
 }
 
@@ -389,6 +411,31 @@ sub parse_diff {
 	return @hunk;
 }
 
+sub colored_diff_hunk {
+	my ($text) = @_;
+	# return the text, so that it can be passed to print()
+	my @ret;
+	for (@$text) {
+		if (!$diff_use_color) {
+			push @ret, $_;
+			next;
+		}
+
+		if (/^\+/) {
+			push @ret, colored($new_color, $_);
+		} elsif (/^\-/) {
+			push @ret, colored($old_color, $_);
+		} elsif (/^\@/) {
+			push @ret, colored($fraginfo_color, $_);
+		} elsif (/^ /) {
+			push @ret, colored($normal_color, $_);
+		} else {
+			push @ret, colored($metainfo_color, $_);
+		}
+	}
+	return @ret;
+}
+
 sub hunk_splittable {
 	my ($text) = @_;
 
@@ -611,9 +658,7 @@ sub patch_update_cmd {
 	my ($ix, $num);
 	my $path = $it->{VALUE};
 	my ($head, @hunk) = parse_diff($path);
-	for (@{$head->{TEXT}}) {
-		print;
-	}
+	print colored_diff_hunk($head->{TEXT});
 	$num = scalar @hunk;
 	$ix = 0;
 
@@ -655,9 +700,7 @@ sub patch_update_cmd {
 		if (hunk_splittable($hunk[$ix]{TEXT})) {
 			$other .= '/s';
 		}
-		for (@{$hunk[$ix]{TEXT}}) {
-			print;
-		}
+		print colored_diff_hunk($hunk[$ix]{TEXT});
 		print colored $prompt_color, "Stage this hunk [y/n/a/d$other/?]? ";
 		my $line = <STDIN>;
 		if ($line) {
@@ -795,8 +838,7 @@ sub diff_cmd {
 				     HEADER => $status_head, },
 				   @mods);
 	return if (!@them);
-	system(qw(git diff-index -p --cached HEAD --),
-	       map { $_->{VALUE} } @them);
+	system(qw(git diff -p --cached HEAD --), map { $_->{VALUE} } @them);
 }
 
 sub quit_cmd {
-- 
1.5.3.5.565.gf0b83-dirty

^ permalink raw reply related

* Re: git svn dcommit with a dirty index
From: Eric Wong @ 2007-11-11  1:42 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: git list
In-Reply-To: <DBC1E504-9007-4C89-9728-2DDBAFF2053B@lrde.epita.fr>

Benoit Sigoure <tsuna@lrde.epita.fr> wrote:
> Hello list,
> From what I understand, when using dcommit, git-svn uses rebase to  
> "sync" the history with what has just been committed.  If the index  
> is in a dirty state, this will cause trouble.  I thought about using  
> git-stash and then git stash apply --index but I'm afraid this could  
> be confusing if dcommit actually brings more revision in that the  
> ones it has just committed.  I'm not sure this is possible and even  
> if it is, it might not be troublesome since if the commits are  
> accepted in the SVN repo, they surely don't overlap with commits that  
> have been sent in the mean time.  But it's risky, so I don't know  
> what to do.  If we use the stash approach, we might want to tell the  
> user that we bailed out because of a problem that needs to be fixed  
> and that he can recover his changes with git stash apply --index.
> 
> Or we should simply check that the index isn't dirty beforehand and  
> refuse to dcommit if it is.
> 
> Any suggestion?

The latter option is much simpler.  I actually thought there was already
a check in dcommit that prevents it from committing with a dirty index,
but apparently not...

> PS OT: Eric, have you made any progress on the svn:externals<- 
> >submodules mapping?  I badly need this feature, but I don't want to  
> start to work on it if you're currently working on it (or about to  
> deal with it) to avoid unecessary effort duplication.

Oops, sorry.  I've been busy and forgetful.  I'll try to work on it
later tonight or tomorrow.

-- 
Eric Wong

^ permalink raw reply

* Re: Deprecate git-fetch-pack?
From: Ask Bjørn Hansen @ 2007-11-11  3:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Daniel Barkalow, git
In-Reply-To: <7v4pftip42.fsf@gitster.siamese.dyndns.org>


On Nov 10, 2007, at 4:48 PM, Junio C Hamano wrote:

> As to peek-remote, ls-remote over the native transport is a
> synonym so I do not think anybody doing the scripting would have
> problem doing s/peek-/ls-/ to their script, but again I do not
> see a heavy maintenance burden to keep the synonym, yet.

For new users the superfluous programs are a burden making it harder  
to learn how everything works.

"What does this thing do?  How is it different from that other similar  
program?  How do they interact?  Oh, this one isn't really used.  Grrh."

At least a note in the documentation that it's superseded by  
$other_program would be helpful for us newcomers.  :-)


  - ask

-- 
http://develooper.com/ - http://askask.com/

^ permalink raw reply

* [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
From: Dan Zwell @ 2007-11-11  0:02 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Shawn O. Pearce, Wincent Colaiuta,
	Git Mailing List, Jonathan del Strother, Johannes Schindelin,
	Frank Lichtenheld
In-Reply-To: <20071104054305.GA13929@sigill.intra.peff.net>

Colors are specified in color.interactive.{prompt,header,help}.
They are specified as git color strings as described in the
documentation. The method color_to_ansi_string() in Git.pm parses
these strings and returns ANSI color codes (using
Term::ANSIColor).

Signed-off-by: Dan Zwell <dzwell@zwell.net>
---
 Documentation/config.txt  |    7 +++++
 git-add--interactive.perl |   19 ++++++++++-----
 perl/Git.pm               |   55
++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 74
insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3712d6a..47c1ab2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -388,6 +388,13 @@ color.interactive::
 	`auto`, use colors only when the output is to the
 	terminal. Defaults to false.
 
+color.interactive.<slot>::
+	Use customized color for `git add --interactive`
+	output. `<slot>` may be `prompt`, `header`, or `help`, for
+	three distinct types of normal output from interactive
+	programs.  The values of these variables may be specified as
+	in color.branch.<slot>.
+
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index f2b0e56..508531f 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1,6 +1,7 @@
 #!/usr/bin/perl -w
 
 use strict;
+use Git;
 
 my ($use_color, $prompt_color, $header_color, $help_color,
$normal_color); my $color_config = qx(git config --get
color.interactive); @@ -8,12 +9,18 @@ if ($color_config=~/true|always/
|| -t STDOUT && $color_config=~/auto/) { eval { require
Term::ANSIColor; }; if (!$@) {
 		$use_color = 1;
-
-		# Sane (visible) defaults:
-		$prompt_color = Term::ANSIColor::color("blue bold");
-		$header_color = Term::ANSIColor::color("bold");
-		$help_color   = Term::ANSIColor::color("red bold");
-		$normal_color = Term::ANSIColor::color("reset");
+		# Set interactive colors:
+
+		# Grab the 3 main colors in git color string format,
with sane
+		# (visible) defaults:
+		my $repo = Git->repository();
+		$prompt_color = Git::color_to_ansi_code(
+			Git::config($repo, "color.interactive.prompt")
|| "bold blue");
+		$header_color = Git::color_to_ansi_code(
+			Git::config($repo, "color.interactive.header")
|| "bold");
+		$help_color = Git::color_to_ansi_code(
+			Git::config($repo, "color.interactive.help")
|| "red bold");
+		$normal_color = Git::color_to_ansi_code("normal");
 	}
 }
 
diff --git a/perl/Git.pm b/perl/Git.pm
index dca92c8..c9e661a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -515,7 +515,6 @@ sub config {
 	};
 }
 
-
 =item config_bool ( VARIABLE )
 
 Retrieve the bool configuration C<VARIABLE>. The return value
@@ -550,6 +549,60 @@ sub config_bool {
 }
 
 
+=item color_to_ansi_code ( COLOR )
+
+Converts a git-style color string, like "underline blue white" to
+an ANSI color code. The code is generated by Term::ANSIColor,
+after the string is parsed into the format that is accepted by
+that module. Used as follows:
+
+	print color_to_ansi_code("underline blue white");
+	print "some text";
+	print color_to_ansi_code("normal");
+
+=cut
+
+sub color_to_ansi_code {
+	my ($git_string) = @_;
+	my @ansi_words;
+	my %attrib_mappings = (
+		"bold"    => "bold",
+		"ul"      => "underline",
+		"blink"   => "blink",
+		# not supported:
+		#"dim"     => "",
+		"reverse" => "reverse"
+	);
+	my ($fg_done, $word);
+
+	foreach $word (split /\s+/, $git_string) {
+		if ($word =~ /normal/) {
+			$fg_done = "true";
+		}
+		elsif ($word =~ /black|red|green|yellow/ ||
+			   $word =~ /blue|magenta|cyan|white/) {
+			# is a color.
+			if ($fg_done) {
+				# this is the background
+				push @ansi_words, "on_" . $word;
+			}
+			else {
+				# this is foreground
+				$fg_done = "true";
+				push @ansi_words, $word;
+			}
+		}
+		else {
+			# this is an attribute, not a color.
+			if ($attrib_mappings{$word}) {
+				push(@ansi_words,
+					 $attrib_mappings{$word});
+			}
+		}
+	}
+	return Term::ANSIColor::color(join(" ", @ansi_words)||"reset");
+}
+
 =item ident ( TYPE | IDENTSTR )
 
 =item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
-- 
1.5.3.5.565.gf0b83-dirty

^ permalink raw reply related

* Re: tracking remotes with Git
From: Ivan Shmakov @ 2007-11-11  4:05 UTC (permalink / raw)
  To: git; +Cc: Ivan Shmakov
In-Reply-To: <200711092138.56277.robin.rosenberg.lists@dewire.com>

>>>>> Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:

[...]

 >> * it looks like `git-cvsimport' uses its own CVS protocol
 >> implementation which doesn't support compression; I've tried to
 >> clone a repository of a project hosted in CVS since circa 1998 and
 >> it 20 MiB or so to obtain revisions until 2000 or so; any ways to
 >> minimize traffic?

 > You can pass options to cvsps.  My guess is -P "-Z" will do it.

	Well, this helps somewhat.  But still, IIUC, cvsps(1) is used
	only to reconstruct the ``patch sets'', and to fetch the actual
	revisions, `git-cvsimport' contacts the CVS repository directly:

--cut: $ nl -ba git-cvsimport--
...
   182	package CVSconn;
   183	# Basic CVS dialog.
   184	# We're only interested in connecting and downloading, so ...
   185	
... not a word about the compression...
   482	package main;
   483	
   484	my $cvs = CVSconn->new($opt_d, $cvs_tree);
...
   911			print "Fetching $fn   v $rev\n" if $opt_v;
   912			my ($tmpname, $size) = $cvs->file($fn,$rev);
...
   930			unlink($tmpname);
...
--cut: $ nl -ba git-cvsimport--

^ permalink raw reply

* [PATCH] fix index-pack with packs >4GB containing deltas on 32-bit machines
From: Nicolas Pitre @ 2007-11-11  4:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This probably hasn't been properly tested before.  Here's a script to 
create a 8GB repo with the necessary characteristics (copy the 
test-genrandom executable from the Git build tree to /tmp first):

-----
#!/bin/bash

git init
git config core.compression 0

# create big objects with no deltas
for i in $(seq -w 1 2 63)
do
	echo $i
	/tmp/test-genrandom $i 268435456 > file_$i
	git add file_$i
	rm file_$i
	echo "file_$i -delta" >> .gitattributes
done

# create "deltifiable" objects in between big objects
for i in $(seq -w 2 2 64)
do
	echo "$i $i $i" >> grow
	cp grow file_$i
	git add file_$i
	rm file_$i
done
rm grow

# create a pack with them
git commit -q -m "commit of big objects interlaced with small deltas"
git repack -a -d
-----

Then clone this repo over the Git protocol.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
diff --git a/index-pack.c b/index-pack.c
index 469a330..9fd6982 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -256,7 +256,7 @@ static void *unpack_raw_entry(struct object_entry *obj, union delta_base *delta_
 
 static void *get_data_from_pack(struct object_entry *obj)
 {
-	unsigned long from = obj[0].idx.offset + obj[0].hdr_size;
+	off_t from = obj[0].idx.offset + obj[0].hdr_size;
 	unsigned long len = obj[1].idx.offset - from;
 	unsigned long rdy = 0;
 	unsigned char *src, *data;

^ permalink raw reply related

* Re: git packs
From: Nicolas Pitre @ 2007-11-11  4:35 UTC (permalink / raw)
  To: bob; +Cc: git
In-Reply-To: <B20E1D71-BCDB-4189-952F-3B809A342870@mac.com>

On Sat, 10 Nov 2007, bob wrote:

> I will try a few things and see if I can get a script put together
> that generates the inflate problem.  The data that I am
> using is a backup of my original repository.  So, I can
> play all that I want.  But it would be a lot easier if I
> could just generate some files using dd or something.

Please see the patch I just posted to the list.  That should fix your 
problem.  I even included a small script to create a repository 
confirming the problem and allowing to test the fix.


Nicolas

^ permalink raw reply

* Subject: [PATCH 1/3] Added basic color support to git add --interactive
From: Dan Zwell @ 2007-11-11  2:23 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Shawn O. Pearce, Wincent Colaiuta,
	Git Mailing List, Jonathan del Strother, Johannes Schindelin,
	Frank Lichtenheld
In-Reply-To: <20071104054305.GA13929@sigill.intra.peff.net>

Added function "colored()" that prints text with a color that
is passed in. Converted many calls to "print" to being calls to
"print colored".

The prompt, the header, and the help output are the 3 types of
colorized output, and each has its own color.

Colorization is done through Term::ANSIColor, which is included
with modern versions of perl. This is optional, and should not
need to be present if color.interactive is not turned on.

Signed-off-by: Dan Zwell <dzwell@zwell.net>
---
The last version of this e-mail was line wrapped, if it was 
successfully sent at all...
 Documentation/config.txt  |    6 ++++++
 git-add--interactive.perl |   44 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8d5d200..3712d6a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -382,6 +382,12 @@ color.diff.<slot>::
 	whitespace).  The values of these variables may be specified as
 	in color.branch.<slot>.
 
+color.interactive::
+	When true (or `always`), always use colors in `git add
+	--interactive`.  When false (or `never`), never.  When set to
+	`auto`, use colors only when the output is to the
+	terminal. Defaults to false.
+
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index ac598f8..f2b0e56 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -2,6 +2,38 @@
 
 use strict;
 
+my ($use_color, $prompt_color, $header_color, $help_color, $normal_color);
+my $color_config = qx(git config --get color.interactive);
+if ($color_config=~/true|always/ || -t STDOUT && $color_config=~/auto/) {
+	eval { require Term::ANSIColor; };
+	if (!$@) {
+		$use_color = 1;
+
+		# Sane (visible) defaults:
+		$prompt_color = Term::ANSIColor::color("blue bold");
+		$header_color = Term::ANSIColor::color("bold");
+		$help_color   = Term::ANSIColor::color("red bold");
+		$normal_color = Term::ANSIColor::color("reset");
+	}
+}
+
+sub colored {
+	my $color = shift;
+	my $string = join("", @_);
+
+	if ($use_color) {
+		# Put a color code at the beginning of each line, a reset at the end
+		# color after newlines that are not at the end of the string
+		$string =~ s/(\n+)(.)/$1$color$2/g;
+		# reset before newlines
+		$string =~ s/(\n+)/$normal_color$1/g;
+		# codes at beginning and end (if necessary):
+		$string =~ s/^/$color/;
+		$string =~ s/$/$normal_color/ unless $string =~ /\n$/;
+	}
+	return $string;
+}
+
 sub run_cmd_pipe {
 	if ($^O eq 'MSWin32') {
 		my @invalid = grep {m/[":*]/} @_;
@@ -175,7 +207,7 @@ sub list_and_choose {
 			if (!$opts->{LIST_FLAT}) {
 				print "     ";
 			}
-			print "$opts->{HEADER}\n";
+			print colored $header_color, "$opts->{HEADER}\n";
 		}
 		for ($i = 0; $i < @stuff; $i++) {
 			my $chosen = $chosen[$i] ? '*' : ' ';
@@ -205,7 +237,7 @@ sub list_and_choose {
 
 		return if ($opts->{LIST_ONLY});
 
-		print $opts->{PROMPT};
+		print colored $prompt_color, $opts->{PROMPT};
 		if ($opts->{SINGLETON}) {
 			print "> ";
 		}
@@ -544,7 +576,7 @@ sub coalesce_overlapping_hunks {
 }
 
 sub help_patch_cmd {
-	print <<\EOF ;
+	print colored $help_color, <<\EOF ;
 y - stage this hunk
 n - do not stage this hunk
 a - stage this and all the remaining hunks
@@ -619,7 +651,7 @@ sub patch_update_cmd {
 		for (@{$hunk[$ix]{TEXT}}) {
 			print;
 		}
-		print "Stage this hunk [y/n/a/d$other/?]? ";
+		print colored $prompt_color, "Stage this hunk [y/n/a/d$other/?]? ";
 		my $line = <STDIN>;
 		if ($line) {
 			if ($line =~ /^y/i) {
@@ -673,7 +705,7 @@ sub patch_update_cmd {
 			elsif ($other =~ /s/ && $line =~ /^s/) {
 				my @split = split_hunk($hunk[$ix]{TEXT});
 				if (1 < @split) {
-					print "Split into ",
+					print colored $header_color, "Split into ",
 					scalar(@split), " hunks.\n";
 				}
 				splice(@hunk, $ix, 1,
@@ -766,7 +798,7 @@ sub quit_cmd {
 }
 
 sub help_cmd {
-	print <<\EOF ;
+	print colored $help_color, <<\EOF ;
 status        - show paths with changes
 update        - add working tree state to the staged set of changes
 revert        - revert staged set of changes back to the HEAD version
-- 
1.5.3.5.565.gf0b83-dirty

^ permalink raw reply related

* Subject: [PATCH 2/3] Let git-add--interactive read colors from .gitconfig
From: Dan Zwell @ 2007-11-11  2:23 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Shawn O. Pearce, Wincent Colaiuta,
	Git Mailing List, Jonathan del Strother, Johannes Schindelin,
	Frank Lichtenheld
In-Reply-To: <20071104054305.GA13929@sigill.intra.peff.net>

Colors are specified in color.interactive.{prompt,header,help}.
They are specified as git color strings as described in the
documentation. The method color_to_ansi_string() in Git.pm parses
these strings and returns ANSI color codes (using
Term::ANSIColor).

Signed-off-by: Dan Zwell <dzwell@zwell.net>
---
The last version of this e-mail was line wrapped, if it was 
successfully sent at all.

 Documentation/config.txt  |    7 +++++
 git-add--interactive.perl |   19 ++++++++++-----
 perl/Git.pm               |   55 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3712d6a..47c1ab2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -388,6 +388,13 @@ color.interactive::
 	`auto`, use colors only when the output is to the
 	terminal. Defaults to false.
 
+color.interactive.<slot>::
+	Use customized color for `git add --interactive`
+	output. `<slot>` may be `prompt`, `header`, or `help`, for
+	three distinct types of normal output from interactive
+	programs.  The values of these variables may be specified as
+	in color.branch.<slot>.
+
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index f2b0e56..508531f 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1,6 +1,7 @@
 #!/usr/bin/perl -w
 
 use strict;
+use Git;
 
 my ($use_color, $prompt_color, $header_color, $help_color, $normal_color);
 my $color_config = qx(git config --get color.interactive);
@@ -8,12 +9,18 @@ if ($color_config=~/true|always/ || -t STDOUT && $color_config=~/auto/) {
 	eval { require Term::ANSIColor; };
 	if (!$@) {
 		$use_color = 1;
-
-		# Sane (visible) defaults:
-		$prompt_color = Term::ANSIColor::color("blue bold");
-		$header_color = Term::ANSIColor::color("bold");
-		$help_color   = Term::ANSIColor::color("red bold");
-		$normal_color = Term::ANSIColor::color("reset");
+		# Set interactive colors:
+
+		# Grab the 3 main colors in git color string format, with sane
+		# (visible) defaults:
+		my $repo = Git->repository();
+		$prompt_color = Git::color_to_ansi_code(
+			Git::config($repo, "color.interactive.prompt") || "bold blue");
+		$header_color = Git::color_to_ansi_code(
+			Git::config($repo, "color.interactive.header") || "bold");
+		$help_color = Git::color_to_ansi_code(
+			Git::config($repo, "color.interactive.help") || "red bold");
+		$normal_color = Git::color_to_ansi_code("normal");
 	}
 }
 
diff --git a/perl/Git.pm b/perl/Git.pm
index dca92c8..c9e661a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -515,7 +515,6 @@ sub config {
 	};
 }
 
-
 =item config_bool ( VARIABLE )
 
 Retrieve the bool configuration C<VARIABLE>. The return value
@@ -550,6 +549,60 @@ sub config_bool {
 }
 
 
+=item color_to_ansi_code ( COLOR )
+
+Converts a git-style color string, like "underline blue white" to
+an ANSI color code. The code is generated by Term::ANSIColor,
+after the string is parsed into the format that is accepted by
+that module. Used as follows:
+
+	print color_to_ansi_code("underline blue white");
+	print "some text";
+	print color_to_ansi_code("normal");
+
+=cut
+
+sub color_to_ansi_code {
+	my ($git_string) = @_;
+	my @ansi_words;
+	my %attrib_mappings = (
+		"bold"    => "bold",
+		"ul"      => "underline",
+		"blink"   => "blink",
+		# not supported:
+		#"dim"     => "",
+		"reverse" => "reverse"
+	);
+	my ($fg_done, $word);
+
+	foreach $word (split /\s+/, $git_string) {
+		if ($word =~ /normal/) {
+			$fg_done = "true";
+		}
+		elsif ($word =~ /black|red|green|yellow/ ||
+			   $word =~ /blue|magenta|cyan|white/) {
+			# is a color.
+			if ($fg_done) {
+				# this is the background
+				push @ansi_words, "on_" . $word;
+			}
+			else {
+				# this is foreground
+				$fg_done = "true";
+				push @ansi_words, $word;
+			}
+		}
+		else {
+			# this is an attribute, not a color.
+			if ($attrib_mappings{$word}) {
+				push(@ansi_words,
+					 $attrib_mappings{$word});
+			}
+		}
+	}
+	return Term::ANSIColor::color(join(" ", @ansi_words)||"reset");
+}
+
 =item ident ( TYPE | IDENTSTR )
 
 =item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
-- 
1.5.3.5.565.gf0b83-dirty

^ permalink raw reply related

* Re: gitk/git-gui misfeature: saving the geometry can the window out of reach
From: Shawn O. Pearce @ 2007-11-11  5:11 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: Git Mailing List
In-Reply-To: <63F7323C-D90A-4F5E-AE3C-8937455972FD@lrde.epita.fr>

Benoit Sigoure <tsuna@lrde.epita.fr> wrote:
> gitk (and I think git-gui too) save their "geometry" (which includes  
> X/Y position) in ~/.gitk.  So far so good.  The problem is that I  
> often use 2 screens at work (one is the screen of my laptop, the  
> other one is above) and I happen to put my gitk/git-gui windows on  
> the 2nd screen.  When I go back home, I don't have a second screen  
> and gitk/git-gui open their windows out of reach.  This could well be  
> an ugly wart in Apple's port of Tcl/Tk (I have a MacBook with OSX  
> 10.4.10) or a more general problem (didn't try with other OSes).
> 
> git-gui version 0.8.4
> git version 1.5.3.5.654.gdd5ec (tonight's master's HEAD)
> Tcl/Tk version 8.4.7 (Apple's)
> (gitk doesn't support --version!)
> 
> For those struggling with the same problem, here is a quick workaround:
>   gnused -i /geometry/d ~/.gitk

Actually git-gui saves the geometry to .git/config so I'm not sure
why the sed line above would correct git-gui's display issues.  But I
have also noticed this problem on my Mac OS X laptop when running
again after leaving either git-gui or gitk on the external display.

I once started working on a fix for git-gui but put it aside as I
didn't have an external monitor handy.  I haven't hooked one up to
my laptop in months so it would be a bit of work for me to test
and debug.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 01/11] Fix memory leak in traverse_commit_list
From: Shawn O. Pearce @ 2007-11-11  5:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmytnqbu2.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > diff --git a/list-objects.c b/list-objects.c
> > index e5c88c2..713bef9 100644
> > --- a/list-objects.c
> > +++ b/list-objects.c
> > @@ -170,4 +170,11 @@ void traverse_commit_list(struct rev_info *revs,
> >  	}
> >  	for (i = 0; i < objects.nr; i++)
> >  		show_object(&objects.objects[i]);
> > +	free(objects.objects);
> > +	if (revs->pending.nr) {
> > +		revs->pending.nr = 0;
> > +		revs->pending.alloc = 0;
> > +		revs->pending.objects = NULL;
> > +		free(revs->pending.objects);
> > +	}
> >  }
> 
> It is locally verifiable that objects.objects are no longer
> needed after this point, but it made me a bit nervous about
> freeing of revs->pending.objects.
> 
> I think the existing callers are all Ok, but somebody else
> should double check.

There are 5 calllers:

* builtin-fetch.c:

   This one I added with my series.  It doesn't care about the
   pending object list.

* builtin-pack-objects.c:

   This doesn't care about the pending list after the call to
   traverse_commit_list.

* builtin-rev-list.c (2):

   Two calls; the first one is for the bisect case where we print
   bisect stats and then return 0 and the second is the end of
   the program for the non-bisect case.  Neither cares about the
   pending list.

* upload-pack.c:

   This is called in the async thread spawned by upload-pack to
   feed pack-objects.  The last thing the async thread does is run
   traverse_commit_list, at which point it exits.  I actually have
   to wonder why we didn't just teach this trick to pack-objects
   so we could avoid the async complexity here in upload-pack.

So yea, the cleanup here is safe, assuming I didn't make the
extremely obvious leak of setting to NULL then calling free()
(as Dscho pointed out).

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 06/11] git-fetch: Release objects used by a prior transport
From: Shawn O. Pearce @ 2007-11-11  5:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vve8bqcke.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > diff --git a/builtin-fetch.c b/builtin-fetch.c
> > index 847db73..18f123e 100644
> > --- a/builtin-fetch.c
> > +++ b/builtin-fetch.c
> > @@ -337,7 +337,10 @@ static void store_updated_refs(const char *url, struct ref *ref_map)
> >  
> >  static int fetch_refs(struct transport *transport, struct ref *ref_map)
> >  {
> > -	int ret = transport_fetch_refs(transport, ref_map);
> > +	int ret;
> > +
> > +	free_all_objects();
> > +	ret = transport_fetch_refs(transport, ref_map);
> >  	if (!ret)
> >  		store_updated_refs(transport->url, ref_map);
> >  	transport_unlock_pack(transport);
> 
> This sounds a very heavy-handed approach.
> 
> Is it the callers responsibility to know what function does call
> free_all_objects() and makes sure there is no pointer to objects
> obtained before the call that is used after the call returns?

Yea, I guess it is.  That's part of the reason why this usage was
put here in a static function of builtin-fetch.  Its high enough
up in the call stack that nobody above it cares.  Where we would
run into trouble would be if a transport decided to hang onto any
pointers in its data structure between calls to the transport.

Lets just say we don't free the objects; the flags are still all
messed up from any prior user.  Its tricky to reuse the objects
because we don't know what state an object is left in by someone
that ran before us (e.g. an internal call to fetch-pack!).  Its
also tricky when they free the parent list but leave parsed=1;
we can't rebuild the parent pointers!

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
From: Yin Ping @ 2007-11-11  5:30 UTC (permalink / raw)
  To: skimo; +Cc: git
In-Reply-To: <20071110195509.GI2261MdfPADPa@greensroom.kotnet.org>

On Nov 11, 2007 3:55 AM, Sven Verdoolaege <skimo@kotnet.org> wrote:
> On Sun, Nov 11, 2007 at 03:27:43AM +0800, Ping Yin wrote:
> > This commit teaches git status/commit to also show commits of user-cared
>
> Does it?  It looks like you only changed git-commit.
> Shouldn't this be put in wt_status_print, if anywhere?
>
git-commit and git-status correspond to the same script git-commit.sh
> Also, you have some typos:
>
> > +     test -n "$modules" && echo -e "#\n# submodule modifiled: "$modules"\n#"
> [..]
> > +                             echo "  Warn: $name dosn't contains commit $headone"
>
> skimo
>
Oops, i'll fix it



-- 
Ping Yin

^ permalink raw reply

* [PATCH 0/3] Adding colors to git-add--interactive
From: Dan Zwell @ 2007-11-11  0:01 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Shawn O. Pearce, Wincent Colaiuta,
	Git Mailing List, Jonathan del Strother, Johannes Schindelin,
	Frank Lichtenheld
In-Reply-To: <20071104054305.GA13929@sigill.intra.peff.net>

A bit of a recap--this feature was requested by a user a few weeks
ago, and I wrote a short patch that implemented partial coloring. The
main criticisms of this patch were:
- The name of the configuration key to turn on interactive coloring
was not well chosen.
- The color attribute synonyms "clear" and "reset" were used
interchangeably, though "reset" is what the rest of git uses.
- The colors were not user-settable.

When the above were fixed, the new patch garnered the following
criticism:
- The color names accepted in .gitconfig were perl color names (to be
fed to Term::ANSIColor), and this was not consistent with the rest
of git. For example, "red blue ul" would have to be written, "red
on_blue underline".

Fixing this (the colors could not be converted by a regex, but had to
be parsed) was very libraryish, and also a little confusing. I was
given a suggestion or two about how to make it more readable. This
parsing also needed to be added to Git.pm instead of
git-add--interactive.perl.

In the next iteration, all of the above were fixed, but as Jeff King
pointed out, I was printing $color before the beginning of a string,
and printing $clear at the end, which allowed ${COLOR}text\n${RESET},
which looks bad on some terminals.

Last iteration, it was pointed out that print_colored must take a
file handle to pave the way for pager support, and Junio Hamano and Jeff
King both gave suggestions as to how, and Junio sent me few changes
that allow printing colored diffs in addition to prompts. I ended up
making changes so that the two color functions can be used with perl's
print():
print colored($color, "some text")
print color_diff_hunk($hunk)

This way, file handles can be added directly to the print calls, when
support for $PAGER is added.

Let me know if other things need correction.

Dan

^ permalink raw reply

* Re: [PATCH 07/11] git-fetch: Limit automated tag following to only fetched objects
From: Shawn O. Pearce @ 2007-11-11  5:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7ikrrr77.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > We now redefine the rule to be: "tags are fetched if they refer
> > to an object that was just transferred; that is an object that is
> > new to your repository".  This rule is quite simple to understand,
> > you only get a tag if you just got the object it refers to.
> 
> In other words, if I do this:
> 
> 	git fetch git-gui master
> 
> (which should not follow any tags) when your master is a bit
> ahead of a new tag in git-gui I do not have, and then
> immediately afterwards if I do:
> 
> 	git fetch git-gui
> 
> I will not get the new tag followed?
> 
> If that is what the patch does, it feels like a regression.

Yeah, that's what it does, and everyone has stated its a regression
so its not the right change to make here.
 
> The intended behaviour was "when tag following is enabled, they
> are followed if they refer to an object that is reachable from
> your existing refs".
> 
> But this is quite expensive to compute.  If a tag points at a
> blob that is contained inside a commit that is reachable from a
> ref, we would need to grep "git rev-list --objects -all" output
> to find it out.  I do not offhand recall what the scripted
> version did, but I would not be surprised if as an approximation
> we did the auto-following by "does the pointee exist" check.

We just did `git cat-file -t`.  If it passed then we followed
the tag.  Which means we could just do a has_sha1_file() test and
call it a day.  If the object is already reachable from our current
refs we'll only download the tag; if it isn't really reachable but
is dangling in the ODB we'll download possibly a lot of objects.

I was trying to make tag auto-following only work if the tag's
target was already completely available and we'd only need to
download the tag object itself.
 
> What "random behaviour" are you trying to fix? 

Apparently I'm introducing even more random behavior.  It may have
just been a bug in the past.  Or a tag whose referred object wasn't
reachable from a ref but I thought it should have been.  Obviously
my patch series resolves neither.

-- 
Shawn.

^ permalink raw reply

* [PATCH 1/3] Added basic color support to git add --interactive
From: Dan Zwell @ 2007-11-11  0:01 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Shawn O. Pearce, Wincent Colaiuta,
	Git Mailing List, Jonathan del Strother, Johannes Schindelin,
	Frank Lichtenheld
In-Reply-To: <20071104054305.GA13929@sigill.intra.peff.net>

Added function "colored()" that prints text with a color that
is passed in. Converted many calls to "print" to being calls to
"print colored".

The prompt, the header, and the help output are the 3 types of
colorized output, and each has its own color.

Colorization is done through Term::ANSIColor, which is included
with modern versions of perl. This is optional, and should not
need to be present if color.interactive is not turned on.

Signed-off-by: Dan Zwell <dzwell@zwell.net>
---
 Documentation/config.txt  |    6 ++++++
 git-add--interactive.perl |   44
++++++++++++++++++++++++++++++++++++++------ 2 files changed, 44
insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8d5d200..3712d6a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -382,6 +382,12 @@ color.diff.<slot>::
 	whitespace).  The values of these variables may be specified as
 	in color.branch.<slot>.
 
+color.interactive::
+	When true (or `always`), always use colors in `git add
+	--interactive`.  When false (or `never`), never.  When set to
+	`auto`, use colors only when the output is to the
+	terminal. Defaults to false.
+
 color.pager::
 	A boolean to enable/disable colored output when the pager is in
 	use (default is true).
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index ac598f8..f2b0e56 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -2,6 +2,38 @@
 
 use strict;
 
+my ($use_color, $prompt_color, $header_color, $help_color,
$normal_color); +my $color_config = qx(git config --get
color.interactive); +if ($color_config=~/true|always/ || -t STDOUT &&
$color_config=~/auto/) {
+	eval { require Term::ANSIColor; };
+	if (!$@) {
+		$use_color = 1;
+
+		# Sane (visible) defaults:
+		$prompt_color = Term::ANSIColor::color("blue bold");
+		$header_color = Term::ANSIColor::color("bold");
+		$help_color   = Term::ANSIColor::color("red bold");
+		$normal_color = Term::ANSIColor::color("reset");
+	}
+}
+
+sub colored {
+	my $color = shift;
+	my $string = join("", @_);
+
+	if ($use_color) {
+		# Put a color code at the beginning of each line, a
reset at the end
+		# color after newlines that are not at the end of the
string
+		$string =~ s/(\n+)(.)/$1$color$2/g;
+		# reset before newlines
+		$string =~ s/(\n+)/$normal_color$1/g;
+		# codes at beginning and end (if necessary):
+		$string =~ s/^/$color/;
+		$string =~ s/$/$normal_color/ unless $string =~ /\n$/;
+	}
+	return $string;
+}
+
 sub run_cmd_pipe {
 	if ($^O eq 'MSWin32') {
 		my @invalid = grep {m/[":*]/} @_;
@@ -175,7 +207,7 @@ sub list_and_choose {
 			if (!$opts->{LIST_FLAT}) {
 				print "     ";
 			}
-			print "$opts->{HEADER}\n";
+			print colored $header_color,
"$opts->{HEADER}\n"; }
 		for ($i = 0; $i < @stuff; $i++) {
 			my $chosen = $chosen[$i] ? '*' : ' ';
@@ -205,7 +237,7 @@ sub list_and_choose {
 
 		return if ($opts->{LIST_ONLY});
 
-		print $opts->{PROMPT};
+		print colored $prompt_color, $opts->{PROMPT};
 		if ($opts->{SINGLETON}) {
 			print "> ";
 		}
@@ -544,7 +576,7 @@ sub coalesce_overlapping_hunks {
 }
 
 sub help_patch_cmd {
-	print <<\EOF ;
+	print colored $help_color, <<\EOF ;
 y - stage this hunk
 n - do not stage this hunk
 a - stage this and all the remaining hunks
@@ -619,7 +651,7 @@ sub patch_update_cmd {
 		for (@{$hunk[$ix]{TEXT}}) {
 			print;
 		}
-		print "Stage this hunk [y/n/a/d$other/?]? ";
+		print colored $prompt_color, "Stage this hunk
[y/n/a/d$other/?]? "; my $line = <STDIN>;
 		if ($line) {
 			if ($line =~ /^y/i) {
@@ -673,7 +705,7 @@ sub patch_update_cmd {
 			elsif ($other =~ /s/ && $line =~ /^s/) {
 				my @split =
split_hunk($hunk[$ix]{TEXT}); if (1 < @split) {
-					print "Split into ",
+					print colored $header_color,
"Split into ", scalar(@split), " hunks.\n";
 				}
 				splice(@hunk, $ix, 1,
@@ -766,7 +798,7 @@ sub quit_cmd {
 }
 
 sub help_cmd {
-	print <<\EOF ;
+	print colored $help_color, <<\EOF ;
 status        - show paths with changes
 update        - add working tree state to the staged set of changes
 revert        - revert staged set of changes back to the HEAD version
-- 
1.5.3.5.565.gf0b83-dirty

^ permalink raw reply related

* Re: git-gui messes up the diff view on non ASCII characters
From: Shawn O. Pearce @ 2007-11-11  5:59 UTC (permalink / raw)
  To: Michele Ballabio; +Cc: git, Peter Baumann
In-Reply-To: <200711092230.37905.barra_cuda@katamail.com>

Michele Ballabio <barra_cuda@katamail.com> wrote:
> On Friday 09 November 2007, Peter Baumann wrote:
> > I'm managing some UTF-8 encoded LaTeX files in git, which include some
> > non ASCII characters like the german ä,ö and ü. If I view the diff with
> > git-diff on an UTF8 enabled terminal, all looks nice. So does the diff
> > view in gitk after I commited my changes. Only git-gui shows some
> > "strange" characters, so I assume it is an encoding problem.
> > 
> > I have to admit that I'm totally unaware how this should work, but at
> > least I think my configuration is correct here, because otherwise git-diff
> > or gitk would show the same behaviour. Is there anything which could be
> > done to make git-gui happy, too?
> 
> It's a known issue, and already on Shawn's ToDo list. I have to add that
> viewing untracked UTF8 files in git-gui works just fine. Weird.

Cute.  That's because in the untracked case we open the file and
let the platform's chosen encoding be used to convert it into the
text viewer.  In the tracked diff case we force the encoding to
be in binary.

Now gitk works because it assumes the diff is in the same character
encoding as the commit message itself.  Since commit messages are
typically in UTF-8 (as that is the Git default encoding) then a
UTF-8 encoded file shows correctly in gitk.

What's the right behavior here?  Just assume the platform encoding
is correct for the file we are showing and show it?  Assume the
commit encoding configured in i18n.commitencoding is the correct
one for the file content?  Something else?

-- 
Shawn.

^ 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