Git development
 help / color / mirror / Atom feed
* [PATCH] builtin/log.c: Fix an "Using plain integer as NULL pointer" warning
From: Ramsay Jones @ 2011-10-08 16:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GIT Mailing-list


Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Junio,

I wanted to request that you squash this into commit 739453a3
(format-patch: use branch description in cover letter, 21-09-2011).
But, since that's in next ...

Hmm, I should probably start at least building the pu branch so that
I could catch these things earlier ...

ATB,
Ramsay Jones

 builtin/log.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index e80a925..4395f3e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1040,7 +1040,7 @@ static char *find_branch_name(struct rev_info *rev)
 	if (positive < 0)
 		return NULL;
 	strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name);
-	branch = resolve_ref(buf.buf, branch_sha1, 1, 0);
+	branch = resolve_ref(buf.buf, branch_sha1, 1, NULL);
 	if (!branch ||
 	    prefixcmp(branch, "refs/heads/") ||
 	    hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))
-- 
1.7.7

^ permalink raw reply related

* Re: [PATCH 2/3] Fix some "variable might be used uninitialized" warnings
From: Ramsay Jones @ 2011-10-08 16:06 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <4E6FDBA4.6050505@ramsay1.demon.co.uk>

Ramsay Jones wrote:
> Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>>
>>> In particular, gcc complains as follows:
>>>
>>>         CC tree-walk.o
>>>     tree-walk.c: In function `traverse_trees':
>>>     tree-walk.c:347: warning: 'e' might be used uninitialized in this \
>>>         function
>>>
>>>         CC builtin/revert.o
>>>     builtin/revert.c: In function `verify_opt_mutually_compatible':
>>>     builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
>>>         this function
>> Could you also add something to this effect to the commit log message:
>>
>> 	but I have verified that these are gcc being not careful
>> 	enough and they are never used uninitialized.
> 
> see below for the v2 patch.
> 
>> If that is what you indeed have done, that is.
> 
> Indeed. The builtin/revert.c warning is straight-forward, but the tree-walk.c
> warning is somewhat less so! ;-)
> 
> Imagine traverse_trees() (tree-walk.c:324) was called with n == 0 (let's ignore
> the effective calls to xmalloc(0) and xcalloc(0,..) at the start of that function).
> At first blush it looked like 'e' would remain uninitialized in the call to
> prune_traversal() at line 403.  Indeed it *would* be if you ever got to that line.
> However, since the 'mask' variable (set at line 391) remains set to zero at line 401,
> the flow of control leaves the loop before 'e' is used.
> 
> [I don't think traverse_trees() would ever be called with n == 0 anyway; the call
> site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)
> which start from unpack_trees() are protected by "if (len)", where 'len' is unsigned.]

When patches don't even make it to pu I just assume you hate them so much that
there is not much chance of them being applied and simply forget about them.
In this case, since compiler warnings are a bugbear of mine, I'm hoping that
you just forgot about this one ... :-D  [if not, sorry for the noise].

For your convenience, I've attached the patch below (rebased against current master).

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] Fix some "variable might be used uninitialized" warnings

In particular, gcc complains as follows:

        CC tree-walk.o
    tree-walk.c: In function `traverse_trees':
    tree-walk.c:347: warning: 'e' might be used uninitialized in this \
        function

        CC builtin/revert.o
    builtin/revert.c: In function `verify_opt_mutually_compatible':
    builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
        this function

However, I have verified that the analysis performed by gcc was too
conservative and that these variables are not, in fact, used while
uninitialized.

In order to suppress the warnings, we add an NULL pointer initializer
to the declarations of the 'e' and 'opt2' variables.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 builtin/revert.c |    2 +-
 tree-walk.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index ba27cf1..200149e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 
 static void verify_opt_mutually_compatible(const char *me, ...)
 {
-	const char *opt1, *opt2;
+	const char *opt1, *opt2 = NULL;
 	va_list ap;
 
 	va_start(ap, me);
diff --git a/tree-walk.c b/tree-walk.c
index 808bb55..a8d8a66 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
 		unsigned long mask, dirmask;
 		const char *first = NULL;
 		int first_len = 0;
-		struct name_entry *e;
+		struct name_entry *e = NULL;
 		int len;
 
 		for (i = 0; i < n; i++) {
-- 
1.7.7

^ permalink raw reply related

* Re: How pretty is pretty? git cat-file -p inconsistency
From: Jakub Narebski @ 2011-10-08 16:36 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Junio C Hamano, Git Mailing List, Linus Torvalds
In-Reply-To: <4E906292.1020909@drmicha.warpmail.net>

On Sat, 8 Oct 2011, Michael J Gruber wrote:
> Jakub Narebski venit, vidit, dixit 08.10.2011 01:50:

> > Tree object consist of zero or more entries.  Each item consist of mode,
> > filename, and sha1:
> > 
> >   <mode> SPC <filename> NUL <sha1>
> > 
> > where
> > 
> > 1. <mode> is variable-length (!) text (!) containing mode of an
> >    entry. It encodes type of entry: if it is blob (including special
> >    case: symbolic link), tree i.e. directory, or a commit
> >    i.e. submodule.  Does not include leading zeros.
> > 
> > 2. <filename> is variable-length null-terminated ("\0") name of a file
> >    or directory, or name of directory where submodule is attached
> > 
> > 3. <sha1> is 40-bytes _binary_ identifier.
> > 
> > HTH
> 
> It does help, thanks.
> 
> Though I'm beginning to think we have a crazy object format. Not only do
> we have a lot of indirections (like ascii representation of decimal
> representation of length), but we store sha1 as ascii in commit and tag
> objects and as binary in tree objects. Which makes tree objects the only
> unpleasant ones to look at (and parse) in raw form. (I was hoping we can
> dispose of/deprecate cat-file -p in favor of show). Oh well.

Well, actually we have pretty consistent format, i.e. we use textual
format everywhere (textual size of blob instead of some variable-length
integer, textual name of type of object instead of a byte for it, epoch
as a text and not 64bit signed int in some network format, hexadecimal sha1,
space separated (sub)fields)... 

... with the sole exception of tree object, which uses _binary_ sha1.
What was Linus thinking?!?

To have consistency the tree entry should IMVHO look like this

  <textual mode> SPC <filename> NUL <hexadecimal sha1> LF


Nb. with hexadecimal sha-1 everywhere it would be I think possible (if very
very difficult) to move to different hash function: SHA-256, Skein, etc.
I don't know if it is now at all possible...

-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH 9/9] completion: remove broken dead code from __git_heads() and __git_tags()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

__git_heads() was introduced in 5de40f5 (Teach bash about
git-repo-config., 2006-11-27), and __git_tags() in 88e21dc (Teach bash
about completing arguments for git-tag, 2007-08-31).  As their name
suggests, __git_heads() is supposed to list only branches, and
__git_tags() only tags.

Since their introduction both of these functions consist of two
distinct parts.  The first part gets branches or tags, respectively,
from a local repositoty using 'git for-each-ref'.  The second part
queries a remote repository given as argument using 'git ls-remote'.

These remote-querying parts are broken in both functions since their
introduction, because they list both branches and tags from the remote
repository.  (The 'git ls-remote' query is not limited to list only
heads or tags, respectively, and the for loop filtering the query
results prints everything except dereferenced tags.)  This breakage
could be easily fixed by passing the '--heads' or '--tags' options or
appropriate refs patterns to the 'git ls-remote' invocations.

However, that no one noticed this breakage yet is probably not a
coincidence: neither of these two functions were used to query a
remote repository, the remote-querying parts were dead code already
upon thier introduction and remained dead ever since.

Since those parts of code are broken, are and were never used, stop
the bit-rotting and remove them.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   22 ++--------------------
 1 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d7151220..802b703d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -536,42 +536,24 @@ __gitcomp_nl ()
 	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
 }
 
-# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
 __git_heads ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
+	local dir="$(__gitdir)"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/heads
 		return
 	fi
-	for i in $(git ls-remote "${1-}" 2>/dev/null); do
-		case "$is_hash,$i" in
-		y,*) is_hash=n ;;
-		n,*^{}) is_hash=y ;;
-		n,refs/heads/*) is_hash=y; echo "${i#refs/heads/}" ;;
-		n,*) is_hash=y; echo "$i" ;;
-		esac
-	done
 }
 
-# __git_tags accepts 0 or 1 arguments (to pass to __gitdir)
 __git_tags ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
+	local dir="$(__gitdir)"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/tags
 		return
 	fi
-	for i in $(git ls-remote "${1-}" 2>/dev/null); do
-		case "$is_hash,$i" in
-		y,*) is_hash=n ;;
-		n,*^{}) is_hash=y ;;
-		n,refs/tags/*) is_hash=y; echo "${i#refs/tags/}" ;;
-		n,*) is_hash=y; echo "$i" ;;
-		esac
-	done
 }
 
 # __git_refs accepts 0, 1 (to pass to __gitdir), or 2 arguments
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 8/9] completion: fast initial completion for config 'remote.*.fetch' value
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

Refspecs for branches in a remote repository start with 'refs/heads/',
so completing those refspecs with 'git config remote.origin.fetch
<TAB>' always offers 'refs/heads/' first, because that's the unique
part of the possible refspecs.  But it does so only after querying the
remote with 'git ls-remote', which can take a while when the request
goes through some slower network to a remote server.

Don't waste the user's time and offer 'refs/heads/' right away for
'git config remote.origin.fetch <TAB>'.

The reason for putting 'refs/heads/' directly into COMPREPLY instead
of using __gitcomp() is to avoid __gitcomp() adding a trailing space.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 658df3a7..d7151220 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1890,6 +1890,10 @@ _git_config ()
 	remote.*.fetch)
 		local remote="${prev#remote.}"
 		remote="${remote%.fetch}"
+		if [ -z "$cur" ]; then
+			COMPREPLY=("refs/heads/")
+			return
+		fi
 		__gitcomp_nl "$(__git_refs_remotes "$remote")"
 		return
 		;;
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 7/9] completion: improve ls-remote output filtering in __git_refs_remotes()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

This follows suit of a previous patch for __git_refs(): use a
while-read loop and let bash's word splitting get rid of object names
from 'git ls-remote's output.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dc1d5e90..658df3a7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -650,15 +650,10 @@ __git_refs2 ()
 # __git_refs_remotes requires 1 argument (to pass to ls-remote)
 __git_refs_remotes ()
 {
-	local i is_hash=y
-	for i in $(git ls-remote "$1" 'refs/heads/*' 2>/dev/null); do
-		case "$is_hash" in
-		n)
-			is_hash=y
-			echo "$i:refs/remotes/$1/${i#refs/heads/}"
-			;;
-		y) is_hash=n ;;
-		esac
+	local i hash
+	git ls-remote "$1" 'refs/heads/*' 2>/dev/null | \
+	while read hash i; do
+		echo "$i:refs/remotes/$1/${i#refs/heads/}"
 	done
 }
 
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 6/9] completion: query only refs/heads/ in __git_refs_remotes()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

__git_refs_remotes() is used to provide completion for refspecs to set
'remote.*.fetch' config variables for branches on the given remote.
So it's really only interested in refs under 'refs/heads/', but it
queries the remote for all its refs and then filters out all refs
outside of 'refs/heads/'.

Let 'git ls-remote' do the filtering.

Also remove the unused $cmd variable from __git_refs_remotes().

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8d3597e..dc1d5e90 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -650,17 +650,14 @@ __git_refs2 ()
 # __git_refs_remotes requires 1 argument (to pass to ls-remote)
 __git_refs_remotes ()
 {
-	local cmd i is_hash=y
-	for i in $(git ls-remote "$1" 2>/dev/null); do
-		case "$is_hash,$i" in
-		n,refs/heads/*)
+	local i is_hash=y
+	for i in $(git ls-remote "$1" 'refs/heads/*' 2>/dev/null); do
+		case "$is_hash" in
+		n)
 			is_hash=y
 			echo "$i:refs/remotes/$1/${i#refs/heads/}"
 			;;
-		y,*) is_hash=n ;;
-		n,*^{}) is_hash=y ;;
-		n,refs/tags/*) is_hash=y;;
-		n,*) is_hash=y; ;;
+		y) is_hash=n ;;
 		esac
 	done
 }
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 5/9] completion: support full refs from remote repositories
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

When the __git_refs() completion helper function lists refs from a
local repository, it usually lists the refs' short name, except when
it needs to provide completion for words starting with refs, because
in that case it lists full ref names, see 608efb87 (bash: complete
full refs, 2008-11-28).

Add the same functionality to the code path dealing with remote
repositories, too.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c6ab742d..a8d3597e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -615,14 +615,27 @@ __git_refs ()
 		fi
 		return
 	fi
-	git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
-	while read hash i; do
-		case "$i" in
-		*^{}) ;;
-		refs/*) echo "${i#refs/*/}" ;;
-		*) echo "$i" ;;
-		esac
-	done
+	case "$cur" in
+	refs|refs/*)
+		git ls-remote "$dir" "$cur*" 2>/dev/null | \
+		while read hash i; do
+			case "$i" in
+			*^{}) ;;
+			*) echo "$i" ;;
+			esac
+		done
+		;;
+	*)
+		git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
+		while read hash i; do
+			case "$i" in
+			*^{}) ;;
+			refs/*) echo "${i#refs/*/}" ;;
+			*) echo "$i" ;;
+			esac
+		done
+		;;
+	esac
 }
 
 # __git_refs2 requires 1 argument (to pass to __git_refs)
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 4/9] completion: improve ls-remote output filtering in __git_refs()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

The remote-handling part of __git_refs() has a nice for loop and state
machine case statement to iterate over all words from the output of
'git ls-remote' to identify object names and ref names.  Since each
line in the output of 'git ls-remote' consists of an object name and a
ref name, we can do more effective filtering by using a while-read
loop and letting bash's word splitting take care of object names.
This way the code is easier to understand and the loop will need only
half the number of iterations than before.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6b5dc5cd..c6ab742d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -579,7 +579,7 @@ __git_tags ()
 # by checkout for tracking branches
 __git_refs ()
 {
-	local i is_hash=y dir="$(__gitdir "${1-}")" track="${2-}"
+	local i hash dir="$(__gitdir "${1-}")" track="${2-}"
 	local format refs
 	if [ -d "$dir" ]; then
 		case "$cur" in
@@ -615,12 +615,12 @@ __git_refs ()
 		fi
 		return
 	fi
-	for i in $(git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null); do
-		case "$is_hash,$i" in
-		y,*) is_hash=n ;;
-		n,*^{}) is_hash=y ;;
-		n,refs/*) is_hash=y; echo "${i#refs/*/}" ;;
-		n,*) is_hash=y; echo "$i" ;;
+	git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null | \
+	while read hash i; do
+		case "$i" in
+		*^{}) ;;
+		refs/*) echo "${i#refs/*/}" ;;
+		*) echo "$i" ;;
 		esac
 	done
 }
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 3/9] completion: make refs completion consistent for local and remote repos
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

For a local repository the __git_refs() completion helper function
lists refs under 'refs/(tags|heads|remotes)/', plus some special refs
like HEAD and ORIG_HEAD.  For a remote repository, however, it lists
all refs.

Fix this inconsistency by specifying refs filter patterns for 'git
ls-remote' to only list refs under 'refs/(tags|heads|remotes)/'.

For now this makes it impossible to complete refs outside of
'refs/(tags|heads|remotes)/' in a remote repository, but a followup
patch will resurrect that.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 86de0bf4..6b5dc5cd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -615,13 +615,11 @@ __git_refs ()
 		fi
 		return
 	fi
-	for i in $(git ls-remote "$dir" 2>/dev/null); do
+	for i in $(git ls-remote "$dir" HEAD ORIG_HEAD 'refs/tags/*' 'refs/heads/*' 'refs/remotes/*' 2>/dev/null); do
 		case "$is_hash,$i" in
 		y,*) is_hash=n ;;
 		n,*^{}) is_hash=y ;;
-		n,refs/tags/*) is_hash=y; echo "${i#refs/tags/}" ;;
-		n,refs/heads/*) is_hash=y; echo "${i#refs/heads/}" ;;
-		n,refs/remotes/*) is_hash=y; echo "${i#refs/remotes/}" ;;
+		n,refs/*) is_hash=y; echo "${i#refs/*/}" ;;
 		n,*) is_hash=y; echo "$i" ;;
 		esac
 	done
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 2/9] completion: optimize refs completion
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

After a unique command or option is completed, in most cases it is a
good thing to add a trailing a space, but sometimes it doesn't makes
sense, e.g. when the completed word is an option taking an argument
('--option=') or a configuration section ('core.').  Therefore the
completion script uses the '-o nospace' option to prevent bash from
automatically appending a space to unique completions, and it has the
__gitcomp() function to add that trailing space only when necessary.
See 72e5e989 (bash: Add space after unique command name is completed.,
2007-02-04), 78d4d6a2 (bash: Support unique completion on git-config.,
2007-02-04), and b3391775 (bash: Support unique completion when
possible., 2007-02-04).

__gitcomp() therefore iterates over all possible completion words it
got as argument, and checks each word whether a trailing space is
necessary or not.  This is ok for commands, options, etc., i.e. when
the number of words is relatively small, but can be noticeably slow
for large number of refs.  However, while options might or might not
need that trailing space, refs are always handled uniformly and always
get that trailing space (or a trailing '.' for 'git config
branch.<head>.').  Since refs listed by __git_refs() & co. are
separated by newline, this allows us some optimizations with
'compgen'.

So, add a specialized variant of __gitcomp() that only deals with
possible completion words separated by a newline and uniformly appends
the trailing space to all words using 'compgen -S' (or any other
suffix, if specified), so no iteration over all words is done.
Convert all callsites of __gitcomp() where it's called with refs, i.e.
when it gets the output of either __git_refs(), __git_heads(),
__git_tags(), __git_refs2(), __git_refs_remotes(), or the odd 'git
for-each-ref' somewhere in _git_config().  Also convert callsites
where it gets other uniformly handled newline separated word lists,
i.e. either remotes from __git_remotes(), names of set configuration
variables from __git_config_get_set_variables(), stashes, or commands
and aliases.

Here are some timing results for dealing with 10000 refs.
Before:

  $ refs="$(__git_refs ~/tmp/git/repo-with-10k-refs/)"
  $ time __gitcomp "$refs"

  real	0m1.134s
  user	0m1.060s
  sys	0m0.130s

After:

  $ time __gitcomp_nl "$refs"

  real	0m0.373s
  user	0m0.360s
  sys	0m0.020s

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |  116 +++++++++++++++++++-------------
 1 files changed, 70 insertions(+), 46 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c0fb6e15..86de0bf4 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -512,6 +512,30 @@ __gitcomp ()
 	esac
 }
 
+# Generates completion reply with compgen.
+# It accepts 1 to 4 arguments:
+# 1: List of possible completion words, separated by a single newline.
+# 2: A prefix to be added to each possible completion word (optional).
+# 3: Generate possible completion matches for this word (optional).
+# 4: A suffix to be appended to each possible completion word (optional).
+#    If omitted, a space is appended; if specified but empty, nothing is
+#    appended.
+__gitcomp_nl ()
+{
+	local s=$'\n' IFS=' '$'\t'$'\n'
+	local cur_="$cur" suffix=" "
+
+	if [ $# -gt 2 ]; then
+		cur_="$3"
+		if [ $# -gt 3 ]; then
+			suffix="$4"
+		fi
+	fi
+
+	IFS=$s
+	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
+}
+
 # __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
 __git_heads ()
 {
@@ -716,15 +740,15 @@ __git_complete_revlist_file ()
 	*...*)
 		pfx="${cur_%...*}..."
 		cur_="${cur_#*...}"
-		__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+		__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		;;
 	*..*)
 		pfx="${cur_%..*}.."
 		cur_="${cur_#*..}"
-		__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+		__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		;;
 	*)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		;;
 	esac
 }
@@ -764,7 +788,7 @@ __git_complete_remote_or_refspec ()
 		c=$((++c))
 	done
 	if [ -z "$remote" ]; then
-		__gitcomp "$(__git_remotes)"
+		__gitcomp_nl "$(__git_remotes)"
 		return
 	fi
 	if [ $no_complete_refspec = 1 ]; then
@@ -789,23 +813,23 @@ __git_complete_remote_or_refspec ()
 	case "$cmd" in
 	fetch)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs2 "$remote")" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs2 "$remote")" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		fi
 		;;
 	pull)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		fi
 		;;
 	push)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur_"
+			__gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
 		fi
 		;;
 	esac
@@ -1084,7 +1108,7 @@ _git_archive ()
 		return
 		;;
 	--remote=*)
-		__gitcomp "$(__git_remotes)" "" "${cur##--remote=}"
+		__gitcomp_nl "$(__git_remotes)" "" "${cur##--remote=}"
 		return
 		;;
 	--*)
@@ -1115,7 +1139,7 @@ _git_bisect ()
 
 	case "$subcommand" in
 	bad|good|reset|skip|start)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		;;
 	*)
 		COMPREPLY=()
@@ -1146,9 +1170,9 @@ _git_branch ()
 		;;
 	*)
 		if [ $only_local_ref = "y" -a $has_r = "n" ]; then
-			__gitcomp "$(__git_heads)"
+			__gitcomp_nl "$(__git_heads)"
 		else
-			__gitcomp "$(__git_refs)"
+			__gitcomp_nl "$(__git_refs)"
 		fi
 		;;
 	esac
@@ -1195,7 +1219,7 @@ _git_checkout ()
 		if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
 			track=''
 		fi
-		__gitcomp "$(__git_refs '' $track)"
+		__gitcomp_nl "$(__git_refs '' $track)"
 		;;
 	esac
 }
@@ -1212,7 +1236,7 @@ _git_cherry_pick ()
 		__gitcomp "--edit --no-commit"
 		;;
 	*)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		;;
 	esac
 }
@@ -1266,7 +1290,7 @@ _git_commit ()
 		;;
 	--reuse-message=*|--reedit-message=*|\
 	--fixup=*|--squash=*)
-		__gitcomp "$(__git_refs)" "" "${cur#*=}"
+		__gitcomp_nl "$(__git_refs)" "" "${cur#*=}"
 		return
 		;;
 	--untracked-files=*)
@@ -1297,7 +1321,7 @@ _git_describe ()
 			"
 		return
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 __git_diff_common_options="--stat --numstat --shortstat --summary
@@ -1456,7 +1480,7 @@ _git_grep ()
 		;;
 	esac
 
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_help ()
@@ -1514,7 +1538,7 @@ _git_ls_files ()
 
 _git_ls_remote ()
 {
-	__gitcomp "$(__git_remotes)"
+	__gitcomp_nl "$(__git_remotes)"
 }
 
 _git_ls_tree ()
@@ -1610,7 +1634,7 @@ _git_merge ()
 		__gitcomp "$__git_merge_options"
 		return
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_mergetool ()
@@ -1630,7 +1654,7 @@ _git_mergetool ()
 
 _git_merge_base ()
 {
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_mv ()
@@ -1661,7 +1685,7 @@ _git_notes ()
 	,*)
 		case "${words[cword-1]}" in
 		--ref)
-			__gitcomp "$(__git_refs)"
+			__gitcomp_nl "$(__git_refs)"
 			;;
 		*)
 			__gitcomp "$subcommands --ref"
@@ -1670,7 +1694,7 @@ _git_notes ()
 		;;
 	add,--reuse-message=*|append,--reuse-message=*|\
 	add,--reedit-message=*|append,--reedit-message=*)
-		__gitcomp "$(__git_refs)" "" "${cur#*=}"
+		__gitcomp_nl "$(__git_refs)" "" "${cur#*=}"
 		;;
 	add,--*|append,--*)
 		__gitcomp '--file= --message= --reedit-message=
@@ -1689,7 +1713,7 @@ _git_notes ()
 		-m|-F)
 			;;
 		*)
-			__gitcomp "$(__git_refs)"
+			__gitcomp_nl "$(__git_refs)"
 			;;
 		esac
 		;;
@@ -1717,12 +1741,12 @@ _git_push ()
 {
 	case "$prev" in
 	--repo)
-		__gitcomp "$(__git_remotes)"
+		__gitcomp_nl "$(__git_remotes)"
 		return
 	esac
 	case "$cur" in
 	--repo=*)
-		__gitcomp "$(__git_remotes)" "" "${cur##--repo=}"
+		__gitcomp_nl "$(__git_remotes)" "" "${cur##--repo=}"
 		return
 		;;
 	--*)
@@ -1760,7 +1784,7 @@ _git_rebase ()
 
 		return
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_reflog ()
@@ -1771,7 +1795,7 @@ _git_reflog ()
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
 	else
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 	fi
 }
 
@@ -1853,23 +1877,23 @@ _git_config ()
 {
 	case "$prev" in
 	branch.*.remote)
-		__gitcomp "$(__git_remotes)"
+		__gitcomp_nl "$(__git_remotes)"
 		return
 		;;
 	branch.*.merge)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		return
 		;;
 	remote.*.fetch)
 		local remote="${prev#remote.}"
 		remote="${remote%.fetch}"
-		__gitcomp "$(__git_refs_remotes "$remote")"
+		__gitcomp_nl "$(__git_refs_remotes "$remote")"
 		return
 		;;
 	remote.*.push)
 		local remote="${prev#remote.}"
 		remote="${remote%.push}"
-		__gitcomp "$(git --git-dir="$(__gitdir)" \
+		__gitcomp_nl "$(git --git-dir="$(__gitdir)" \
 			for-each-ref --format='%(refname):%(refname)' \
 			refs/heads)"
 		return
@@ -1916,7 +1940,7 @@ _git_config ()
 		return
 		;;
 	--get|--get-all|--unset|--unset-all)
-		__gitcomp "$(__git_config_get_set_variables)"
+		__gitcomp_nl "$(__git_config_get_set_variables)"
 		return
 		;;
 	*.*)
@@ -1942,7 +1966,7 @@ _git_config ()
 		;;
 	branch.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
-		__gitcomp "$(__git_heads)" "$pfx" "$cur_" "."
+		__gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
 		return
 		;;
 	guitool.*.*)
@@ -1971,7 +1995,7 @@ _git_config ()
 	pager.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
 		__git_compute_all_commands
-		__gitcomp "$__git_all_commands" "$pfx" "$cur_"
+		__gitcomp_nl "$__git_all_commands" "$pfx" "$cur_"
 		return
 		;;
 	remote.*.*)
@@ -1984,7 +2008,7 @@ _git_config ()
 		;;
 	remote.*)
 		local pfx="${cur%.*}." cur_="${cur#*.}"
-		__gitcomp "$(__git_remotes)" "$pfx" "$cur_" "."
+		__gitcomp_nl "$(__git_remotes)" "$pfx" "$cur_" "."
 		return
 		;;
 	url.*.*)
@@ -2285,7 +2309,7 @@ _git_remote ()
 
 	case "$subcommand" in
 	rename|rm|show|prune)
-		__gitcomp "$(__git_remotes)"
+		__gitcomp_nl "$(__git_remotes)"
 		;;
 	update)
 		local i c='' IFS=$'\n'
@@ -2303,7 +2327,7 @@ _git_remote ()
 
 _git_replace ()
 {
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_reset ()
@@ -2316,7 +2340,7 @@ _git_reset ()
 		return
 		;;
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_revert ()
@@ -2327,7 +2351,7 @@ _git_revert ()
 		return
 		;;
 	esac
-	__gitcomp "$(__git_refs)"
+	__gitcomp_nl "$(__git_refs)"
 }
 
 _git_rm ()
@@ -2426,7 +2450,7 @@ _git_stash ()
 			COMPREPLY=()
 			;;
 		show,*|apply,*|drop,*|pop,*|branch,*)
-			__gitcomp "$(git --git-dir="$(__gitdir)" stash list \
+			__gitcomp_nl "$(git --git-dir="$(__gitdir)" stash list \
 					| sed -n -e 's/:.*//p')"
 			;;
 		*)
@@ -2560,7 +2584,7 @@ _git_tag ()
 		i="${words[c]}"
 		case "$i" in
 		-d|-v)
-			__gitcomp "$(__git_tags)"
+			__gitcomp_nl "$(__git_tags)"
 			return
 			;;
 		-f)
@@ -2576,13 +2600,13 @@ _git_tag ()
 		;;
 	-*|tag)
 		if [ $f = 1 ]; then
-			__gitcomp "$(__git_tags)"
+			__gitcomp_nl "$(__git_tags)"
 		else
 			COMPREPLY=()
 		fi
 		;;
 	*)
-		__gitcomp "$(__git_refs)"
+		__gitcomp_nl "$(__git_refs)"
 		;;
 	esac
 }
@@ -2635,7 +2659,7 @@ _git ()
 			"
 			;;
 		*)     __git_compute_porcelain_commands
-		       __gitcomp "$__git_porcelain_commands $(__git_aliases)" ;;
+		       __gitcomp_nl "$__git_porcelain_commands $(__git_aliases)" ;;
 		esac
 		return
 	fi
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 1/9] completion: document __gitcomp()
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor
In-Reply-To: <1318085683-29830-1-git-send-email-szeder@ira.uka.de>

I always forget which argument is which, and got tired of figuring it
out over and over again.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b36f9e70..c0fb6e15 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -485,8 +485,13 @@ _get_comp_words_by_ref ()
 fi
 fi
 
-# __gitcomp accepts 1, 2, 3, or 4 arguments
-# generates completion reply with compgen
+# Generates completion reply with compgen, appending a space to possible
+# completion words, if necessary.
+# It accepts 1 to 4 arguments:
+# 1: List of possible completion words.
+# 2: A prefix to be added to each possible completion word (optional).
+# 3: Generate possible completion matches for this word (optional).
+# 4: A suffix to be appended to each possible completion word (optional).
 __gitcomp ()
 {
 	local cur_="$cur"
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 0/9] ref completion optimizations, fixes, and cleanups
From: SZEDER Gábor @ 2011-10-08 14:54 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Jonathan Nieder, Junio C Hamano,
	SZEDER Gábor

Hi,

This series aims to improve the completion of refs & co.

This one is the most important in the series; it takes some shortcuts
to make completing large number of refs faster (it's also faster for
git.git, but it's unnoticeable).

  [2/9] completion: optimize refs completion

The following three make __git_refs() handle local and remote
repositories more consistently, and also clean up the remote-handling
code part of __git_refs().  They likely make things a bit faster, but
since the code path usually involves network communication I didn't
run any benchmarks.

  [3/9] completion: make refs completion consistent for local and remote
          repos
  [4/9] completion: improve ls-remote output filtering in __git_refs()
  [5/9] completion: support full refs from remote repositories

The following two do similar cleanups in __git_refs_remotes() than 3/9
and 4/9 in __git_refs().

  [6/9] completion: query only refs/heads/ in __git_refs_remotes()
  [7/9] completion: improve ls-remote output filtering in
          __git_refs_remotes()

A silly while-at-it optimization; the delay eliminated by this one was
annoying when testing 6/9 and 7/9.

  [8/9] completion: fast initial completion for config 'remote.*.fetch'
          value

And finally remove some bitrotted code.

  [9/9] completion: remove broken dead code from __git_heads() and
          __git_tags()


This series is meant to be applied on the merge of master and 77653abd
(completion: commit --fixup and --squash, 2011-10-06) from pu, and the
patch in

  Message-ID: <20111008010634.GB11561@goldbirke>
  (http://article.gmane.org/gmane.comp.version-control.git/183131)

from last night applied.  There will be two easily fixable conflicts
when applied directly on top of current master.


Best,
Gábor


SZEDER Gábor (9):
  completion: document __gitcomp()
  completion: optimize refs completion
  completion: make refs completion consistent for local and remote
    repos
  completion: improve ls-remote output filtering in __git_refs()
  completion: support full refs from remote repositories
  completion: query only refs/heads/ in __git_refs_remotes()
  completion: improve ls-remote output filtering in
    __git_refs_remotes()
  completion: fast initial completion for config 'remote.*.fetch' value
  completion: remove broken dead code from __git_heads() and
    __git_tags()

 contrib/completion/git-completion.bash |  200 +++++++++++++++++---------------
 1 files changed, 109 insertions(+), 91 deletions(-)

-- 
1.7.7.187.ga41de

^ permalink raw reply

* Re: How pretty is pretty? git cat-file -p inconsistency
From: Michael J Gruber @ 2011-10-08 14:47 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <m3r52o1hxr.fsf@localhost.localdomain>

Jakub Narebski venit, vidit, dixit 08.10.2011 01:50:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
> [cut]
>> I never knew how ugly the output of "git tag-file tree sha1" is. I guess
>> it's the type of object whose format I don't know... We don't have an
>> object format description in Doc/technical, do we? tree.c doesn't tell
>> me much.
> 
> I had to handle this in my attempt to write "git blame <directory>" in Perl,
> which was using `git cat-file --batch`, and that gives raw data and not
> pretty-printed.
> 
> Tree object consist of zero or more entries.  Each item consist of mode,
> filename, and sha1:
> 
>   <mode> SPC <filename> NUL <sha1>
> 
> where
> 
> 1. <mode> is variable-length (!) text (!) containing mode of an
>    entry. It encodes type of entry: if it is blob (including special
>    case: symbolic link), tree i.e. directory, or a commit
>    i.e. submodule.  Does not include leading zeros.
> 
> 2. <filename> is variable-length null-terminated ("\0") name of a file
>    or directory, or name of directory where submodule is attached
> 
> 3. <sha1> is 40-bytes _binary_ identifier.
> 
> HTH

It does help, thanks.

Though I'm beginning to think we have a crazy object format. Not only do
we have a lot of indirections (like ascii representation of decimal
representation of length), but we store sha1 as ascii in commit and tag
objects and as binary in tree objects. Which makes tree objects the only
unpleasant ones to look at (and parse) in raw form. (I was hoping we can
dispose of/deprecate cat-file -p in favor of show). Oh well.

Michael

^ permalink raw reply

* Display line numbers in gitk?
From: Sebastian Schuberth @ 2011-10-08 13:27 UTC (permalink / raw)
  To: git; +Cc: Pat Thoyts

Hi,

is there currently a way to display line numbers next to each line in 
the diff shown by gitk, e.g. in some kind of gutter?

If that's currently not possible, how much work would it require to add 
that feature as an option?

-- 
Sebastian Schuberth

^ permalink raw reply

* [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
From: Sitaram Chamarty @ 2011-10-08 13:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Phil Hord, Sitaram Chamarty, git
In-Reply-To: <7vwrcgtvh4.fsf@alter.siamese.dyndns.org>

This is useful if you forgot to restrict the diff to the paths you want
to see, or selecting precisely the ones you want is too much typing.

Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>
---

On Fri, Oct 07, 2011 at 01:09:11PM -0700, Junio C Hamano wrote:

> Looks OK from a cursory viewing. Do we want some additional tests?
> 
> For that matter, have you run the test suite with this patch applied (I
> haven't)?

OK; done.  I got some "broken" but nothing "failed":

    make aggregate-results
    make[3]: Entering directory `/home/sitaram/clones/git/t'
    for f in test-results/t*-*.counts; do \
            echo "$f"; \
    done | '/bin/sh' ./aggregate-results.sh
    fixed   0
    success 7377
    failed  0
    broken  49
    total   7461

Hope that is not a problem.

However, I'm not sure the file names that 'git difftool'
comes up with are in a predictable order.  That would mess
up the test, but I can neither make it fail not find
definitive information on the order in which the changed
files are processed.

 git-difftool--helper.sh |    9 +++++----
 t/t7800-difftool.sh     |   44 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index 8452890..0468446 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -38,15 +38,16 @@ launch_merge_tool () {
 
 	# $LOCAL and $REMOTE are temporary files so prompt
 	# the user with the real $MERGED name before launching $merge_tool.
+	ans=y
 	if should_prompt
 	then
 		printf "\nViewing: '$MERGED'\n"
 		if use_ext_cmd
 		then
-			printf "Hit return to launch '%s': " \
+			printf "Launch '%s' [Y/n]: " \
 				"$GIT_DIFFTOOL_EXTCMD"
 		else
-			printf "Hit return to launch '%s': " "$merge_tool"
+			printf "Launch '%s' [Y/n]: " "$merge_tool"
 		fi
 		read ans
 	fi
@@ -54,9 +55,9 @@ launch_merge_tool () {
 	if use_ext_cmd
 	then
 		export BASE
-		eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
+		test "$ans" != "n" && eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
 	else
-		run_merge_tool "$merge_tool"
+		test "$ans" != "n" && run_merge_tool "$merge_tool"
 	fi
 }
 
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 395adfc..f547e0b 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -38,7 +38,18 @@ restore_test_defaults()
 prompt_given()
 {
 	prompt="$1"
-	test "$prompt" = "Hit return to launch 'test-tool': branch"
+	test "$prompt" = "Launch 'test-tool' [Y/n]: branch"
+}
+
+stdin_contains()
+{
+	grep >/dev/null "$1"
+}
+
+stdin_doesnot_contain()
+{
+	grep >/dev/null "$1" && return 1
+	return 0
 }
 
 # Create a file on master and change it on branch
@@ -265,4 +276,35 @@ test_expect_success PERL 'difftool --extcmd cat arg2' '
 	test "$diff" = branch
 '
 
+# Create a second file on master and a different version on branch
+test_expect_success PERL 'setup with 2 files different' '
+	echo m2 >file2 &&
+	git add file2 &&
+	git commit -m "added file2" &&
+
+	git checkout branch &&
+	echo br2 >file2 &&
+	git add file2 &&
+	git commit -a -m "branch changed file2" &&
+	git checkout master
+'
+
+test_expect_success PERL 'say no to the first file' '
+	diff=$((echo n; echo) | git difftool -x cat branch) &&
+
+	echo "$diff" | stdin_contains m2 &&
+	echo "$diff" | stdin_contains br2 &&
+	echo "$diff" | stdin_doesnot_contain master &&
+	echo "$diff" | stdin_doesnot_contain branch
+'
+
+test_expect_success PERL 'say no to the second file' '
+	diff=$((echo; echo n) | git difftool -x cat branch) &&
+
+	echo "$diff" | stdin_contains master &&
+	echo "$diff" | stdin_contains branch &&
+	echo "$diff" | stdin_doesnot_contain m2 &&
+	echo "$diff" | stdin_doesnot_contain br2
+'
+
 test_done
-- 
1.7.6

^ permalink raw reply related

* Re: gitk: 'j' and 'k' keyboard shortcuts backward
From: Paul Mackerras @ 2011-10-08  7:04 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Clemens Buchacher, Josh Triplett, Pat Thoyts, Robert Suetterlin,
	git
In-Reply-To: <20110919164950.GB2861@elie>

On Mon, Sep 19, 2011 at 11:49:50AM -0500, Jonathan Nieder wrote:

> How about this patch?
> 
> -- >8 --
> Subject: gitk: Make vi-style keybindings more vi-like

Thanks, applied, minus the .po file updates.  The translaters seem to
prefer that I leave the .po file updates to them.

Paul.

^ permalink raw reply

* Re: [PATCH v3] gitk: Teach gitk to respect log.showroot
From: Paul Mackerras @ 2011-10-08  6:47 UTC (permalink / raw)
  To: Marcus Karlsson; +Cc: zbyszek, gitster, git
In-Reply-To: <20111004200813.GA16596@kennedy.acc.umu.se>

On Tue, Oct 04, 2011 at 10:08:13PM +0200, Marcus Karlsson wrote:
> Teach gitk to respect log.showroot.

Sounds reasonable, ...

> -	set cmd [concat | git diff-tree -r $flags $ids]
> +	set cmd [concat | git diff-tree -r]
> +	if {$log_showroot eq true} {
> +	    set cmd [concat $cmd --root]
> +	}
> +	set cmd [concat $cmd $flags $ids]

but is there any reason not to do it like this?

	if {$log_showroot} {
	    lappend flags --root
	}
	set cmd [concat | git diff-tree -r $flags $ids]

I.e., do you particularly want the --root before the other flags?

Paul.

^ permalink raw reply

* [PATCH 3/3] refs.c: free duplicate entries in the ref array instead of leaking them
From: Brandon Casey @ 2011-10-08  3:20 UTC (permalink / raw)
  To: git; +Cc: julian, Brandon Casey
In-Reply-To: <3k7giKa3PkJZo51iAXivXCFEZpYY2WC3depjtuvksrCQPax7dSLVCXpMlqLxWtZfSp6P10yMhMg@cipher.nrlssc.navy.mil>

From: Brandon Casey <drafnel@gmail.com>


Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 refs.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/refs.c b/refs.c
index cbc4c5d..df39297 100644
--- a/refs.c
+++ b/refs.c
@@ -94,6 +94,7 @@ static void sort_ref_array(struct ref_array *array)
 				die("Duplicated ref, and SHA1s don't match: %s",
 				    a->name);
 			warning("Duplicated ref: %s", a->name);
+			free(b);
 			continue;
 		}
 		i++;
-- 
1.7.7

^ permalink raw reply related

* [PATCH 2/3] refs.c: abort ref search if ref array is empty
From: Brandon Casey @ 2011-10-08  3:20 UTC (permalink / raw)
  To: git; +Cc: julian, Brandon Casey
In-Reply-To: <3k7giKa3PkJZo51iAXivXCFEZpYY2WC3depjtuvksrCQPax7dSLVCXpMlqLxWtZfSp6P10yMhMg@cipher.nrlssc.navy.mil>

From: Brandon Casey <drafnel@gmail.com>

The bsearch() implementation on IRIX 6.5 segfaults if it is passed NULL
for the base array argument even if number-of-elements is zero.  So, let's
work around it by detecting an empty array and aborting early.

This is a useful optimization in its own right anyway, since we avoid a
useless allocation and initialization of the ref_entry when the ref array
is empty.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 refs.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/refs.c b/refs.c
index c31b461..cbc4c5d 100644
--- a/refs.c
+++ b/refs.c
@@ -110,6 +110,9 @@ static struct ref_entry *search_ref_array(struct ref_array *array, const char *n
 	if (name == NULL)
 		return NULL;
 
+	if (!array->nr)
+		return NULL;
+
 	len = strlen(name) + 1;
 	e = xmalloc(sizeof(struct ref_entry) + len);
 	memcpy(e->name, name, len);
-- 
1.7.7

^ permalink raw reply related

* [PATCH 1/3] refs.c: ensure struct whose member may be passed to realloc is initialized
From: Brandon Casey @ 2011-10-08  3:20 UTC (permalink / raw)
  To: git; +Cc: julian, Brandon Casey
In-Reply-To: <3k7giKa3PkJZo51iAXivXCFEZpYY2WC3depjtuvksrCQPax7dSLVCXpMlqLxWtZfSp6P10yMhMg@cipher.nrlssc.navy.mil>

From: Brandon Casey <drafnel@gmail.com>

The variable "refs" is allocated on the stack but is not initialized.  It
is passed to read_packed_refs(), and its struct members may eventually be
passed to add_ref() and ALLOC_GROW().  Since the structure has not been
initialized, its members may contain random non-zero values.  So let's
initialize it.

The call sequence looks something like this:

   resolve_gitlink_packed_ref(...) {

       struct cached_refs refs;
       ...
       read_packed_refs(f, &refs);
       ...
   }

   read_packed_refs(FILE*, struct cached_refs *cached_refs) {
       ...
       add_ref(name, sha1, flag, &cached_refs->packed, &last);
       ...
   }

   add_ref(..., struct ref_array *refs, struct ref_entry **) {
       ...
       ALLOC_GROW(refs->refs, refs->nr + 1, refs->alloc);
   }

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 refs.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/refs.c b/refs.c
index 5835b40..c31b461 100644
--- a/refs.c
+++ b/refs.c
@@ -360,6 +360,7 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen, const char *refna
 	f = fopen(name, "r");
 	if (!f)
 		return -1;
+	memset(&refs, 0, sizeof(refs));
 	read_packed_refs(f, &refs);
 	fclose(f);
 	ref = search_ref_array(&refs.packed, refname);
-- 
1.7.7

^ permalink raw reply related

* [PATCH 0/3] jp/get-ref-dir-unsorted fixups
From: Brandon Casey @ 2011-10-08  3:20 UTC (permalink / raw)
  To: git; +Cc: julian

Here are a few fixups I found while investigating a segfault on IRIX.

The first patch fixes a potential segfault due to an uninitialized
variable, but it has already been made moot by Michael Haggerty's
updates to refs.c that are merged in next.  So, not sure if it's worth
applying or not.

The second one fixes the segfault on IRIX and is a valid thing to do
anyway.

The third one plugs a little memory leak that may never occur.

Built on top of jp/get-ref-dir-unsorted e9c4c111.

-Brandon

Brandon Casey (3):
  refs.c: ensure struct whose member may be passed to realloc is
    initialized
  refs.c: abort ref search if ref array is empty
  refs.c: free duplicate entries in the ref array instead of leaking
    them

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

-- 
1.7.7

^ permalink raw reply

* Re: unexpected behavior with `git log --skip filename`
From: Andrew McNabb @ 2011-10-08  2:36 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <CAG+J_DwnUOeDTiUW-UUJGLLg8jJ4EhXN21B7o_hOMnyowM9a8g@mail.gmail.com>

On Fri, Oct 07, 2011 at 05:54:36PM -0400, Jay Soffian wrote:
> 
> Hmm:
> 
> $ git log --oneline GIT-VERSION-GEN | head -2
> 7f41b6bbe3 Post 1.7.7 first wave
> 703f05ad58 Git 1.7.7
> 
> $ git log --oneline --skip=1 -n 1 GIT-VERSION-GEN
> 703f05ad58 Git 1.7.7

I went back to reproduce this, and I think I may have been using the
--follow option earlier.  In my private repository, git log gives
identical output for the last two commits when I don't specify --skip:

$ git log -n 2 --oneline httpd.conf.orig
f0026e9 updated many of the *.orig files to the latest version
e57e840 moved the .orig files into place, too
$ git log --follow -n 2 --oneline httpd.conf.orig
f0026e9 updated many of the *.orig files to the latest version
e57e840 moved the .orig files into place, too
$

But when I specify --skip=1, the output is different:

$ git log -n 1 --skip=1 --oneline httpd.conf.orig
e57e840 moved the .orig files into place, too
$ git log --follow -n 1 --skip=1 --oneline httpd.conf.orig
f0026e9 updated many of the *.orig files to the latest version
$


GIT-VERSION-GEN example that you shared, I don't notice this difference.
It's not immediately obvious to me what's different between the two
examples.

--
Andrew McNabb
http://www.mcnabbs.org/andrew/
PGP Fingerprint: 8A17 B57C 6879 1863 DE55  8012 AB4D 6098 8826 6868

^ permalink raw reply

* [PATCH 2/2] completion: unite --format and --pretty for 'log' and 'show'
From: SZEDER Gábor @ 2011-10-08  1:09 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce; +Cc: Teemu Matilainen, git
In-Reply-To: <20111008010432.GA11561@goldbirke>

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   18 ++++--------------
 1 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 98282435..b36f9e70 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1553,14 +1553,9 @@ _git_log ()
 		merge="--merge"
 	fi
 	case "$cur" in
-	--pretty=*)
+	--pretty=*|--format=*)
 		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur##--pretty=}"
-		return
-		;;
-	--format=*)
-		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur##--format=}"
+			" "" "${cur#*=}"
 		return
 		;;
 	--date=*)
@@ -2365,14 +2360,9 @@ _git_show ()
 	__git_has_doubledash && return
 
 	case "$cur" in
-	--pretty=*)
-		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur##--pretty=}"
-		return
-		;;
-	--format=*)
+	--pretty=*|--format=*)
 		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
-			" "" "${cur##--format=}"
+			" "" "${cur#*=}"
 		return
 		;;
 	--*)
-- 
1.7.7.187.ga41de

^ permalink raw reply related

* [PATCH 1/2] completion: unite --reuse-message and --reedit-message for 'notes'
From: SZEDER Gábor @ 2011-10-08  1:06 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce; +Cc: Teemu Matilainen, git
In-Reply-To: <20111008010432.GA11561@goldbirke>

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 847e6e9a..98282435 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1668,11 +1668,9 @@ _git_notes ()
 			;;
 		esac
 		;;
-	add,--reuse-message=*|append,--reuse-message=*)
-		__gitcomp "$(__git_refs)" "" "${cur##--reuse-message=}"
-		;;
+	add,--reuse-message=*|append,--reuse-message=*|\
 	add,--reedit-message=*|append,--reedit-message=*)
-		__gitcomp "$(__git_refs)" "" "${cur##--reedit-message=}"
+		__gitcomp "$(__git_refs)" "" "${cur#*=}"
 		;;
 	add,--*|append,--*)
 		__gitcomp '--file= --message= --reedit-message=
-- 
1.7.7.187.ga41de

^ permalink raw reply related


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