Git development
 help / color / mirror / Atom feed
* [PATCH 3/4] remote: refactor code into alloc_delete_ref()
From: Felipe Contreras @ 2012-02-22 22:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1329950621-21165-1-git-send-email-felipe.contreras@gmail.com>

Will be useful in next patches. No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 remote.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/remote.c b/remote.c
index 9d29acc..4709e20 100644
--- a/remote.c
+++ b/remote.c
@@ -978,16 +978,20 @@ static void tail_link_ref(struct ref *ref, struct ref ***tail)
 	*tail = &ref->next;
 }
 
+static struct ref *alloc_delete_ref(void)
+{
+	struct ref *ref = alloc_ref("(delete)");
+	hashclr(ref->new_sha1);
+	return ref;
+}
+
 static struct ref *try_explicit_object_name(const char *name)
 {
 	unsigned char sha1[20];
 	struct ref *ref;
 
-	if (!*name) {
-		ref = alloc_ref("(delete)");
-		hashclr(ref->new_sha1);
-		return ref;
-	}
+	if (!*name)
+		return alloc_delete_ref();
 	if (get_sha1(name, sha1))
 		return NULL;
 	ref = alloc_ref(name);
-- 
1.7.9.1

^ permalink raw reply related

* [PATCH 4/4] push: add 'prune' option
From: Felipe Contreras @ 2012-02-22 22:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1329950621-21165-1-git-send-email-felipe.contreras@gmail.com>

This will allow us to remove refs from the remote that have been removed
locally.

It's useful to conveniently synchronize all the local branches to
certain remote.

Unfortunately, if we want to handle src:dst refspecs properly, some
extra complexity is needed, so check_pattern_match() needs to be
reorganized (previous patch).

This ensures that the following works:

 $ git push --prune remote refs/heads/*:refs/tmp/*

So that local branches are not only pushed as custom refs in the remote,
but also refs/tmp/foo would be removed if there's no corresponding local
refs/heads/foo.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-push.txt |    9 ++++++++-
 builtin/push.c             |    2 ++
 remote.c                   |   31 ++++++++++++++++++++++++++++---
 remote.h                   |    3 ++-
 t/t5516-fetch-push.sh      |   16 ++++++++++++++++
 transport.c                |    2 ++
 transport.h                |    1 +
 7 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index aede488..204f36d 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git push' [--all | --mirror | --tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
-	   [--repo=<repository>] [-f | --force] [-v | --verbose] [-u | --set-upstream]
+	   [--repo=<repository>] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream]
 	   [<repository> [<refspec>...]]
 
 DESCRIPTION
@@ -71,6 +71,13 @@ nor in any Push line of the corresponding remotes file---see below).
 	Instead of naming each ref to push, specifies that all
 	refs under `refs/heads/` be pushed.
 
+--prune::
+	Remove remote branches that don't have a local counterpart. For example
+	a remote branch `tmp` will be removed if a local branch with the same
+	name doesn't exist any more. This also respects refspecs, e.g.
+	`refs/heads/*:refs/tmp/*` would make sure that remote `refs/tmp/foo`
+	will be removed if `refs/heads/foo` doesn't exist.
+
 --mirror::
 	Instead of naming each ref to push, specifies that all
 	refs under `refs/` (which includes but is not
diff --git a/builtin/push.c b/builtin/push.c
index 35cce53..fdfb2c4 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -261,6 +261,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BIT('u', "set-upstream", &flags, "set upstream for git pull/status",
 			TRANSPORT_PUSH_SET_UPSTREAM),
 		OPT_BOOLEAN(0, "progress", &progress, "force progress reporting"),
+		OPT_BIT(0, "prune", &flags, "prune locally removed refs",
+			TRANSPORT_PUSH_PRUNE),
 		OPT_END()
 	};
 
diff --git a/remote.c b/remote.c
index 4709e20..9c590ae 100644
--- a/remote.c
+++ b/remote.c
@@ -8,6 +8,8 @@
 #include "tag.h"
 #include "string-list.h"
 
+enum map_direction { FROM_SRC, FROM_DST };
+
 static struct refspec s_tag_refspec = {
 	0,
 	1,
@@ -1115,7 +1117,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
 }
 
 static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref *ref,
-		int send_mirror, const struct refspec **ret_pat)
+		int send_mirror, int direction, const struct refspec **ret_pat)
 {
 	const struct refspec *pat;
 	char *name;
@@ -1130,7 +1132,12 @@ static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref
 
 		if (rs[i].pattern) {
 			const char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;
-			if (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {
+			int match;
+			if (direction == FROM_SRC)
+				match = match_name_with_pattern(rs[i].src, ref->name, dst_side, &name);
+			else
+				match = match_name_with_pattern(dst_side, ref->name, rs[i].src, &name);
+			if (match) {
 				matching_refs = i;
 				break;
 			}
@@ -1177,6 +1184,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
 	struct refspec *rs;
 	int send_all = flags & MATCH_REFS_ALL;
 	int send_mirror = flags & MATCH_REFS_MIRROR;
+	int send_prune = flags & MATCH_REFS_PRUNE;
 	int errs;
 	static const char *default_refspec[] = { ":", NULL };
 	struct ref *ref, **dst_tail = tail_ref(dst);
@@ -1197,7 +1205,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
 		if (ref->peer_ref)
 			continue;
 
-		dst_name = get_ref_match(rs, nr_refspec, ref, send_mirror, &pat);
+		dst_name = get_ref_match(rs, nr_refspec, ref, send_mirror, FROM_SRC, &pat);
 		if (!dst_name)
 			continue;
 
@@ -1224,6 +1232,23 @@ int match_push_refs(struct ref *src, struct ref **dst,
 	free_name:
 		free(dst_name);
 	}
+	if (send_prune) {
+		/* check for missing refs on the remote */
+		for (ref = *dst; ref; ref = ref->next) {
+			char *src_name;
+
+			if (ref->peer_ref)
+				/* We're already sending something to this ref. */
+				continue;
+
+			src_name = get_ref_match(rs, nr_refspec, ref, send_mirror, FROM_DST, NULL);
+			if (src_name) {
+				if (!find_ref_by_name(src, src_name))
+					ref->peer_ref = alloc_delete_ref();
+				free(src_name);
+			}
+		}
+	}
 	if (errs)
 		return -1;
 	return 0;
diff --git a/remote.h b/remote.h
index b395598..341142c 100644
--- a/remote.h
+++ b/remote.h
@@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
 enum match_refs_flags {
 	MATCH_REFS_NONE		= 0,
 	MATCH_REFS_ALL 		= (1 << 0),
-	MATCH_REFS_MIRROR	= (1 << 1)
+	MATCH_REFS_MIRROR	= (1 << 1),
+	MATCH_REFS_PRUNE	= (1 << 2),
 };
 
 /* Reporting of tracking info */
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b69cf57..b5417cc 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -979,4 +979,20 @@ test_expect_success 'push --porcelain --dry-run rejected' '
 	test_cmp .git/foo .git/bar
 '
 
+test_expect_success 'push --prune' '
+	mk_test heads/master heads/second heads/foo heads/bar &&
+	git push --prune testrepo &&
+	check_push_result $the_commit heads/master &&
+	check_push_result $the_first_commit heads/second &&
+	! check_push_result $the_first_commit heads/foo heads/bar
+'
+
+test_expect_success 'push --prune refspec' '
+	mk_test tmp/master tmp/second tmp/foo tmp/bar &&
+	git push --prune testrepo "refs/heads/*:refs/tmp/*" &&
+	check_push_result $the_commit tmp/master &&
+	check_push_result $the_first_commit tmp/second &&
+	! check_push_result $the_first_commit tmp/foo tmp/bar
+'
+
 test_done
diff --git a/transport.c b/transport.c
index cac0c06..c20267c 100644
--- a/transport.c
+++ b/transport.c
@@ -1028,6 +1028,8 @@ int transport_push(struct transport *transport,
 			match_flags |= MATCH_REFS_ALL;
 		if (flags & TRANSPORT_PUSH_MIRROR)
 			match_flags |= MATCH_REFS_MIRROR;
+		if (flags & TRANSPORT_PUSH_PRUNE)
+			match_flags |= MATCH_REFS_PRUNE;
 
 		if (match_push_refs(local_refs, &remote_refs,
 				    refspec_nr, refspec, match_flags)) {
diff --git a/transport.h b/transport.h
index 059b330..ce99ef8 100644
--- a/transport.h
+++ b/transport.h
@@ -102,6 +102,7 @@ struct transport {
 #define TRANSPORT_PUSH_PORCELAIN 16
 #define TRANSPORT_PUSH_SET_UPSTREAM 32
 #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64
+#define TRANSPORT_PUSH_PRUNE 128
 
 #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
 
-- 
1.7.9.1

^ permalink raw reply related

* [PATCH 1/4] remote: use a local variable in match_push_refs()
From: Felipe Contreras @ 2012-02-22 22:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1329950621-21165-1-git-send-email-felipe.contreras@gmail.com>

So that we can reuse src later on. No functional changes.

Will be useful in next patches.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 remote.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/remote.c b/remote.c
index 73a3809..55d68d1 100644
--- a/remote.c
+++ b/remote.c
@@ -1157,7 +1157,7 @@ int match_push_refs(struct ref *src, struct ref **dst,
 	int send_mirror = flags & MATCH_REFS_MIRROR;
 	int errs;
 	static const char *default_refspec[] = { ":", NULL };
-	struct ref **dst_tail = tail_ref(dst);
+	struct ref *ref, **dst_tail = tail_ref(dst);
 
 	if (!nr_refspec) {
 		nr_refspec = 1;
@@ -1167,14 +1167,14 @@ int match_push_refs(struct ref *src, struct ref **dst,
 	errs = match_explicit_refs(src, *dst, &dst_tail, rs, nr_refspec);
 
 	/* pick the remainder */
-	for ( ; src; src = src->next) {
+	for (ref = src; ref; ref = ref->next) {
 		struct ref *dst_peer;
 		const struct refspec *pat = NULL;
 		char *dst_name;
-		if (src->peer_ref)
+		if (ref->peer_ref)
 			continue;
 
-		pat = check_pattern_match(rs, nr_refspec, src);
+		pat = check_pattern_match(rs, nr_refspec, ref);
 		if (!pat)
 			continue;
 
@@ -1184,13 +1184,14 @@ int match_push_refs(struct ref *src, struct ref **dst,
 			 * including refs outside refs/heads/ hierarchy, but
 			 * that does not make much sense these days.
 			 */
-			if (!send_mirror && prefixcmp(src->name, "refs/heads/"))
+			if (!send_mirror && prefixcmp(ref->name, "refs/heads/"))
 				continue;
-			dst_name = xstrdup(src->name);
+			dst_name = xstrdup(ref->name);
+
 
 		} else {
 			const char *dst_side = pat->dst ? pat->dst : pat->src;
-			if (!match_name_with_pattern(pat->src, src->name,
+			if (!match_name_with_pattern(pat->src, ref->name,
 						     dst_side, &dst_name))
 				die("Didn't think it matches any more");
 		}
@@ -1211,9 +1212,9 @@ int match_push_refs(struct ref *src, struct ref **dst,
 
 			/* Create a new one and link it */
 			dst_peer = make_linked_ref(dst_name, &dst_tail);
-			hashcpy(dst_peer->new_sha1, src->new_sha1);
+			hashcpy(dst_peer->new_sha1, ref->new_sha1);
 		}
-		dst_peer->peer_ref = copy_ref(src);
+		dst_peer->peer_ref = copy_ref(ref);
 		dst_peer->force = pat->force;
 	free_name:
 		free(dst_name);
-- 
1.7.9.1

^ permalink raw reply related

* Re: Manual hunk edit mode + emacs + ^G == garbage
From: Matt McClure @ 2012-02-22 21:43 UTC (permalink / raw)
  To: git
In-Reply-To: <EDCC7CB3-4DFF-45B8-9E23-E12045CC29D7@sb.org>

Kevin Ballard <kevin <at> sb.org> writes:

> I'm still very much interested in finding a solution to why ^G kills
> emacs when it's invoked by git. As I said earlier, it appears that emacs
> shares the same process group with the perl process that called it, even
> though Andreas Schwab says it puts itself into its own process group and
> invoking emacs from the shell does just that. Does anyone know why this
> might be happening?

Kevin,

I was having the same problem you described on Mac OS X Lion and found this
old thread. A workaround that works for me is:

~/bin/emacs.sh:

#!/bin/bash -i

emacs "$@"

-- cut --

git config --global core.editor ~/bin/emacs.sh

Matt

^ permalink raw reply

* Re: [RFC/PATCH 3/3] push: add 'prune' option
From: Felipe Contreras @ 2012-02-22 23:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <CAMP44s3+XCM1E_AtW1yGifmGoGSkFSpSTaFbbMffz+hmUzWahw@mail.gmail.com>

On Wed, Feb 22, 2012 at 10:43 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Feb 18, 2012 at 12:34 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Does it make sense to specify --prune when --mirror is in effect?  If so,
>> how would it behave differently from a vanilla --mirror?  If not, should
>> it be detected as an error?
>
> Probably doesn't make sense, should be an error.

Actually, after writing the patch for the documentation I realized
this would be difficult to describe: you can use --all --prune, but
not --mirror --prune, and the documentation currently has '[--all |
--mirror | --tags]'. So I decided to make it orthogonal, you can use
--prune with --all, --tags, *and* --mirror. Of course, using --prune
--mirror is the same as --mirror.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCHv3 1/5] refs: add match_pattern()
From: Tom Grennan @ 2012-02-22 23:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, peff, jasampler, pclouds
In-Reply-To: <7vobsrbcny.fsf@alter.siamese.dyndns.org>

On Tue, Feb 21, 2012 at 10:33:05PM -0800, Junio C Hamano wrote:
>Tom Grennan <tmgrennan@gmail.com> writes:
>
>> +static int match_path(const char *name, const char *pattern, int nlen)
>> +{
>> +	int plen = strlen(pattern);
>> +
>> +	return ((plen <= nlen) &&
>> +		!strncmp(name, pattern, plen) &&
>> +		(name[plen] == '\0' ||
>> +		 name[plen] == '/' ||
>> +		 pattern[plen-1] == '/'));
>> +}
>
>This is a counterpart to the tail match found in ls-remote, so we would
>want to call it with a name that makes it clear this is a leading path
>match not just "path" match.  Perhaps match_leading_path() or something.

OK

>> +int match_pattern(const char *name, const char **match,
>> +		  struct string_list *exclude, int flags)
>> +{
>> +	int nlen = strlen(name);
>> +
>> +	if (exclude) {
>> +		struct string_list_item *x;
>> +		for_each_string_list_item(x, exclude) {
>> +			if (!fnmatch(x->string, name, 0))
>> +				return 0;
>> +		}
>> +	}
>> +	if (!match || !*match)
>> +		return 1;
>> +	for (; *match; match++) {
>> +		if (flags == FNM_PATHNAME)
>> +			if (match_path(name, *match, nlen))
>> +				return 1;
>> +		if (!fnmatch(*match, name, flags))
>> +			return 1;
>> +	}
>> +	return 0;
>> +}
>
>As an API for a consolidated and generic function, the design needs a bit
>more improving, I would think.
>
> - The name match_pattern() was OK for a static function inside a single
>   file, but it is way too vague for a global function. This is to match
>   refnames, so I suspect there should at least be a string "ref_"
>   somewhere in its name.

OK

> - You pass "flags" argument, so that later we _could_ enhance the
>   implementation to cover needs for new callers, but alas, it uses its
>   full bits to express only one "do we do FNM_PATHNAME or not?" bit of
>   information, so essentially "flags" does not give us any expandability.

I agree.

> - Is it a sane assumption that a caller that asks FNM_PATHNAME will
>   always want match_path() semantics, too?  Aren't these two logically
>   independent?

Yes, these should be ligically independent although the current use has
combined them.

> - Is it a sane assumption that a caller that gives an exclude list will
>   want neither FNM_PATHNAME semantics nor match_path() semantics?

I'm not sure.  I tried using FNM_PATHNAME with both exclusion and match
patterns of git-for-each-ref but I couldn't get it to do something like
this:
	git for-each-ref ... --exclude '*HEAD' refs/remotes/

I don't remember if this worked,
	git for-each-ref ... --exclude HEAD refs/remotes/

Now I see how an implicit TRAILING match would be useful,
	git for-each-ref ... --exclude /HEAD refs/remotes/

Where git-for-each-ref uses this flag:
	REF_MATCH_LEADING | REF_MATCH_TRAILING | REF_MATCH_FNM_PATH

I'll experiment with this more. 

> - Positive patterns are passed in "const char **match", and negative ones
>   are in "struct string_list *". Doesn't the inconsistency strike you as
>   strange?

Yes, I tried to minimize change but the conversion of argv's to
string_list's won't add that much.

>Perhaps like...
>
>#define REF_MATCH_LEADING       01
>#define REF_MATCH_TRAILING      02
>#define REF_MATCH_FNM_PATH      04
>
>static int match_one(const char *name, size_t namelen, const char *pattern,
>		unsigned flags)
>{
>       	if ((flags & REF_MATCH_LEADING) &&
>            match_leading_path(name, pattern, namelen))
>		return 1;
>       	if ((flags & REF_MATCH_TRAILING) &&
>            match_trailing_path(name, pattern, namelen))
>		return 1;
>	if (!fnmatch(pattern, name, 
>		     (flags & REF_MATCH_FNM_PATH) ? FNM_PATHNAME : 0))
>		return 1;
>	return 0;
>}
>
>int ref_match_pattern(const char *name,
>		const char **pattern, const char **exclude, unsigned flags)
>{
>	size_t namelen = strlen(name);
>        if (exclude) {
>		while (*exclude) {
>			if (match_one(name, namelen, *exclude, flags))
>				return 0;
>			exclude++;
>		}
>	}
>        if (!pattern || !*pattern)
>        	return 1;
>	while (*pattern) {
>		if (match_one(name, namelen, *pattern, flags))
>			return 1;
>		pattern++;
>	}
>        return 0;
>}
>
>and then the caller could do something like
>
>	ref_match_pattern("refs/heads/master",
>        		  ["maste?", NULL],
>                          ["refs/heads/", NULL],
>                          (REF_MATCH_FNM_PATH|REF_MATCH_LEADING));
>
>Note that the above "ref_match_pattern()" gives the same "flags" for the
>call to match_one() for elements in both positive and negative array and
>it is very deliberate.  See review comment to [3/5] for the reasoning.

OK, I think that I understand, but please confirm, you'd expect no output in
the above example, right?

-- 
TomG

^ permalink raw reply

* Re: [PATCH v2] contrib: added git-diffall
From: Junio C Hamano @ 2012-02-22 23:48 UTC (permalink / raw)
  To: Tim Henigan; +Cc: git, davvid, stefano.lattarini
In-Reply-To: <1329948749-5908-1-git-send-email-tim.henigan@gmail.com>

Tim Henigan <tim.henigan@gmail.com> writes:

> This script adds directory diff support to git.  It launches a single
> instance of the user-configured external diff tool and performs a
> directory diff between the specified revisions. The before/after files
> are copied to a tmp directory to do this.  Either 'diff.tool' or
> 'merge.tool' must be set before running the script.
>
> The existing 'git difftool' command already allows the user to view diffs
> using an external tool.  However if multiple files have changed, a
> separate instance of the tool is launched for each one.  This can be
> tedious when many files are involved.

We encourage our log messages to describe the problem first and then
present solution to the problem, so I would update the above perhaps like
this:

	The 'git difftool' command lets the user to use an external tool
	to view diffs, but it runs the tool for one file at the time. This
	makes it tedious to review a change that spans multiple files.

        The "git-diffall" script instead prepares temporary directories
        with preimage and postimage files, and launches a single instance
        of an external diff tool to view the differences in them.
        diff.tool or merge.tool configuration variable is used to specify
        what external tool is used.

I am wondering if reusing "diff.tool" or "merge.tool" is a good idea,
though.

I guess that it is OK to assume that any external tool that can compare
two directories MUST be able to compare two individual files, and if that
is true, it is perfectly fine to reuse the configuration.  But if an
external tool "frobdiff" that can compare two directories cannot compare
two individual files, it will make it impossible for the user to run "git
difftool" if diff.tool is set to "frobdiff" to use with "diffall".

Another thing that comes to my mind is if a user has an external tool that
can use "diffall", is there ever a situation where the user chooses to use
"difftool" instead, to go files one by one.  I cannot offhand imagine any.

Perhaps a longer term integration plan may be to lift the logic from this
script and make it part of "difftool", and then add a boolean variable
"difftool.<tool>.canCompareDirectory", without adding "git diffall" as a
subcommand.  The user can still run "git difftool", and when the external
tool can compare two directories, the code to populate temporary directory
(and to set the trap to clean after itself) taken from this tool will run
inside "git difftool" frontend and then the external tool to compare the
two directories is spawned.

I also think that in a yet longer term, if this mode of "instantiate two
directories to be compared, and let the external tool do the comparison"
proves useful, almost all the "interesting" work done in this script
should be made unnecessary by adding an updated "external diff interface"
on the core side, so that nobody has to hurt his brain to implement an
error-prone command line parsing logic.

In other words, this statement cannot stay true:

> +This script is compatible with all the forms used to specify a
> +range of revisions to diff:

without updating the script every time underlying "git diff" gains new way
of comparing things.  If we move the "prepare two directories, and point
an external tool at them" logic to the core, we do not have to worry about
it at all.

Besides, I do not think the script covers all the forms; "git diff -R"
support is totally missing.

But that is all two steps in the future.

> +# mktemp is not available on all platforms (missing from msysgit)
> +# Use a hard-coded tmp dir if it is not available
> +tmp="$(mktemp -d -t tmp.XXXXXX 2>/dev/null)" || {
> +	tmp=/tmp/git-diffall-tmp
> +}

It would not withstand malicious attacks, but doing

	tmp=/tmp/git-diffall-tmp.$$

would at least protect you from accidental name crashes better in the
fallback codepath.

> +
> +trap 'rm -rf "$tmp" 2>/dev/null' EXIT

Do you need to suppress errors, especially when you are running "rm -rf"
with the 'f' option?

> +mkdir -p "$tmp"
> +
> +left=
> +right=
> +paths=
> +path_sep=
> +compare_staged=
> +common_ancestor=
> +left_dir=
> +right_dir=
> +diff_tool=
> +copy_back=

You can write multiple assignment on a line to save vertical space if you
want to, and the initialization sequence like this is a good place to do
so.

> +while test $# != 0
> +do
> +	case "$1" in
> +	-h|--h|--he|--hel|--help)
> +		usage
> +		;;
> +	--cached)
> +		compare_staged=1
> +		;;
> +	--copy-back)
> +		copy_back=1
> +		;;
> +	-x|--e|--ex|--ext|--extc|--extcm|--extcmd)
> +		diff_tool=$2
> +		shift
> +		;;

What if your command line ends with -x without $2?

Don't you want to match "-t/--tool" that "difftool" already uses?

> +	--)
> +		path_sep=1
> +		;;
> +	-*)
> +		echo Invalid option: "$1"
> +		usage
> +		;;
> +	*)
> +		# could be commit, commit range or path limiter
> +		case "$1" in
> +		*...*)
> +			left=${1%...*}
> +			right=${1#*...}
> +			common_ancestor=1
> +			;;

Strictly speaking, that is not just a common_ancestor but is a merge_base,
which is a common ancestor none of whose children is a common ancestor.

> +		*..*)
> +			left=${1%..*}
> +			right=${1#*..}
> +			;;
> +		*)
> +			if test -n "$path_sep"
> +			then
> +				paths="$paths$1 "
> +			elif test -z "$left"
> +			then
> +				left=$1
> +			elif test -z "$right"
> +			then
> +				right=$1
> +			else
> +				paths="$paths$1 "
> +			fi
> +			;;
> +		esac

Hrm, so "diffall HEAD~2 Documentation/" is not the way to compare the
contents of the Documentation/ directory between the named commit and
the working tree, like "diff HEAD~2 Documentation/" does.

That is not a show-stopper (a double-dash is an easy workaround), but
it is worth pointing out.

> +		;;
> +	esac
> +	shift
> +done
> +
> +# Determine the set of files which changed
> +if test -n "$left" && test -n "$right"
> +then
> +	left_dir="cmt-$(git rev-parse --short $left)"
> +	right_dir="cmt-$(git rev-parse --short $right)"
> +
> +	if test -n "$compare_staged"
> +	then
> +		usage
> +	elif test -n "$common_ancestor"
> +	then
> +		git diff --name-only "$left"..."$right" -- $paths > "$tmp/filelist"
> +	else
> +		git diff --name-only "$left" "$right" -- $paths > "$tmp/filelist"
> +	fi

And this will not work with pathspec that have $IFS characters.  If we
really wanted to we could do that by properly quoting "$1" when you build
$paths and then use eval when you run "git diff" here (look for 'sq' and
'eval' in existing scripts, e.g. "git-am.sh", if you are interested).

Also you may want to write filelist using -z format to protect yourself
from paths that contain LF, but that would require the loop "while read
name" to be rewritten.

> +# Exit immediately if there are no diffs
> +if test ! -s "$tmp/filelist"
> +then
> +	exit 0
> +fi

Ok, you have trap set already so $tmp will disappear with this exit ;-)

> +if test -n "$copy_back" && test "$right_dir" != "working_tree"
> +then
> +	echo "--copy-back is only valid when diff includes the working tree."
> +	exit 1
> +fi

I actually wondered why $right_dir needs to be populated with a copy in
the first place (if you do not copy but give the working tree itself to
the external tool, you do not even have to copy back).

I know the answer to the question, namely, "because the external tool
thinks files that are not in $left_dir are added files", but if you can
find a way to tell the external tool to ignore new files (similar to how
"diff -r" without -N works), running the tool with temporary left_dir and
the true workdir as right_dir would be a lot cleaner solution to the
problem.

> +# Create the named tmp directories that will hold the files to be compared
> +mkdir -p "$tmp/$left_dir" "$tmp/$right_dir"
> +
> +# Populate the tmp/right_dir directory with the files to be compared
> +if test -n "$right"
> +then
> +	while read name
> +	do
> +		ls_list=$(git ls-tree $right $name)
> +		if test -n "$ls_list"
> +		then
> +			mkdir -p "$tmp/$right_dir/$(dirname "$name")"
> +			git show "$right":"$name" > "$tmp/$right_dir/$name" || true
> +		fi
> +	done < "$tmp/filelist"

"while read -r name" might make this slightly more robust; even though
this loses leading and trailing whitespaces in filenames, we probably
can get away without worrying about them.

> +else
> +	# Mac users have gnutar rather than tar
> +	(tar --ignore-failed-read -c -T "$tmp/filelist" | (cd "$tmp/$right_dir" && tar -x)) || {
> +		gnutar --ignore-failed-read -c -T "$tmp/filelist" | (cd "$tmp/$right_dir" && gnutar -x)
> +	}

What is this "--ignore-failed-read" about?  Not reporting unreadable as an
error smells really bad.

If you require GNUism in your tar usage, this should be made configurable
so that people can use alternative names (some systems come with "tar"
that is POSIX and "gtar" that is GNU).

> +cd "$tmp"
> +LOCAL="$left_dir"
> +REMOTE="$right_dir"
> +
> +if test -n "$diff_tool"
> +then
> +	export BASE
> +	eval $diff_tool '"$LOCAL"' '"$REMOTE"'
> +else
> +	run_merge_tool "$merge_tool" false
> +fi
> +
> +# Copy files back to the working dir, if requested
> +if test -n "$copy_back" && test "$right_dir" = "working_tree"
> +then
> +	cd "$start_dir"
> +	git_top_dir=$(git rev-parse --show-toplevel)
> +	find "$tmp/$right_dir" -type f |
> +	while read file
> +	do
> +		cp "$file" "$git_top_dir/${file#$tmp/$right_dir/}"
> +	done
> +fi

This will copy new files created in $right_dir.  Is that intended?

^ permalink raw reply

* Re: [RFC/PATCH 3/3] push: add 'prune' option
From: Junio C Hamano @ 2012-02-22 23:54 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <CAMP44s0DnVfd_Ged5rnhM4BF1jEtm4jzyq9sJAoEtQ-0CM5_aA@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> ... Of course, using --prune
> --mirror is the same as --mirror.

Sounds good.  Thanks.

^ permalink raw reply

* Re: [PATCHv3 1/5] refs: add match_pattern()
From: Junio C Hamano @ 2012-02-23  0:17 UTC (permalink / raw)
  To: Tom Grennan; +Cc: Junio C Hamano, git, peff, jasampler, pclouds
In-Reply-To: <20120222234733.GD2410@tgrennan-laptop>

Tom Grennan <tmgrennan@gmail.com> writes:

> Yes, I tried to minimize change but the conversion of argv's to
> string_list's won't add that much.

How about _not_ using string_list?  After all, string_list is not just a
collection of strings, but is a table to hold strings with attributes.  I
thought argv_array is more appropriate abstraction for the purpose of your
patch.
>>	ref_match_pattern("refs/heads/master",
>>        		  ["maste?", NULL],
>>                          ["refs/heads/", NULL],
>>                          (REF_MATCH_FNM_PATH|REF_MATCH_LEADING));
>>
>>Note that the above "ref_match_pattern()" gives the same "flags" for the
>>call to match_one() for elements in both positive and negative array and
>>it is very deliberate.  See review comment to [3/5] for the reasoning.
>
> OK, I think that I understand, but please confirm, you'd expect no output in
> the above example, right?

"maste?" would match with FNM_PATHNAME with "refs/heads/master" but
the negative "refs/heads/" matches with it, so yeah, I expect that the
function would return false.

^ permalink raw reply

* Re: [PATCHv3 3/5] tag --exclude option
From: Tom Grennan @ 2012-02-23  0:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, peff, jasampler, pclouds
In-Reply-To: <7vhayjbcna.fsf@alter.siamese.dyndns.org>

On Tue, Feb 21, 2012 at 10:33:29PM -0800, Junio C Hamano wrote:
>Tom Grennan <tmgrennan@gmail.com> writes:
>
>> Example,
>>   $ git tag -l --exclude "*-rc?" "v1.7.8*"
>>   v1.7.8
>>   v1.7.8.1
>>   v1.7.8.2
>>   v1.7.8.3
>>   v1.7.8.4
>>
>> Which is equivalent to,
>>   $ git tag -l "v1.7.8*" | grep -v \\-rc.
>>   v1.7.8
>>   v1.7.8.1
>>   v1.7.8.2
>>   v1.7.8.3
>>   v1.7.8.4
>>
>> Signed-off-by: Tom Grennan <tmgrennan@gmail.com>
>
>Having an example is a good way to illustrate your explanation, but it is
>not a substitution.  Could we have at least one real sentence to describe
>what the added option *does*?
>
>This comment applies to all the patches in this series except for the
>second patch.
>

OK, I'll add the "exclude" option description from the respective man pages.

>> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
>> index 8d32b9a..470bd80 100644
>> --- a/Documentation/git-tag.txt
>> +++ b/Documentation/git-tag.txt
>> @@ -13,7 +13,7 @@ SYNOPSIS
>>  	<tagname> [<commit> | <object>]
>>  'git tag' -d <tagname>...
>>  'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
>> -	[<pattern>...]
>> +	[--exclude <pattern>] [<pattern>...]
>>  'git tag' -v <tagname>...
>>  
>>  DESCRIPTION
>> @@ -90,6 +90,10 @@ OPTIONS
>>  --points-at <object>::
>>  	Only list tags of the given object.
>>  
>> +--exclude <pattern>::
>> +	Don't list tags matching the given pattern.  This has precedence
>> +	over any other match pattern arguments.
>
>As you do not specify what kind of pattern matching is done to this
>exclude pattern, it is important to use the same logic between positive
>and negative ones to give users a consistent UI.  Unfortunately we use
>fnmatch without FNM_PATHNAME for positive ones, so this exclude pattern
>needs to follow the same semantics to reduce confusion.
>
>This comment applies to all the patches in this series to add this option
>to existing commands that take the positive pattern.

OK, should I also describe the --no-exclude option?

>> @@ -202,6 +206,15 @@ test_expect_success \
>>  '
>>  
>>  cat >expect <<EOF
>> +v0.2.1
>> +EOF
>> +test_expect_success \
>> +	'listing tags with a suffix as pattern and prefix exclusion' '
>> +	git tag -l --exclude "v1.*" "*.1" > actual &&
>> +	test_cmp expect actual
>> +'
>
>I know you are imitating the style of surrounding tests that is an older
>parts of this script, but it is an eyesore.  More modern tests are written
>like this:
>
>	test_expect_success 'label for the test' '
>		cat >expect <<-EOF &&
>                v0.2.1
>		EOF
>	        git tag -l ... >actual &&
>		test_cmp expect actual
>	'
>
>to avoid unnecessary backslash on the first line, and have the preparation
>of test vectore _inside_ test_expect_success.  We would eventually want to
>update the older part to the newer style for consistency.
>
>Two possible ways to go about this are (1) have a "pure style" patch at
>the beginning to update older tests to a new style and then add new code
>and new test as a follow-up patch written in modern, or (2) add new code
>and new test in modern, and make a mental note to update the older ones
>after the dust settles.  Adding new tests written in older style to a file
>that already has mixed styles is the worst thing you can do.
>
>This comment applies to all the patches in this series with tests.

I'd prefer, (1) precede each "--exclude" patch with a "pure style" patch
to update the respective tests.  However, since this will result in a
lot of conflict with concurrent development;  I'll separate the test
patches from the code and documentation.  I'll then cycle on rebasing
the style and new test patches until the development of each is
quiescent.

Thanks,
TomG

^ permalink raw reply

* Re: [PATCH 4/4] push: add 'prune' option
From: Junio C Hamano @ 2012-02-23  0:42 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <1329950621-21165-5-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> +--prune::
> +	Remove remote branches that don't have a local counterpart. For example
> +	a remote branch `tmp` will be removed if a local branch with the same
> +	name doesn't exist any more. This also respects refspecs, e.g.
> +	`refs/heads/*:refs/tmp/*` would make sure that remote `refs/tmp/foo`
> +	will be removed if `refs/heads/foo` doesn't exist.

I do not think it adds much useful information to mention `tmp` only once
over what is already said by the first sentence.  Also, the first sentence
of the example does not make it clear that it is assuming a same-for-same
mapping.

Coming up with a precise technical description is easy, but it is hard to
explain to the end user in easy terms, and I commend you for attempting to
add an example in a short single sentence, though.

Perhaps spelling out the underlying assumption the example makes is the
best we could do here without going too technical.

        ... For example, if you are pushing all your local branches to
        update the local branches of the remote, `tmp` branch will be
        removed from the remote if you removed your `tmp` branch locally.

        If you are pushing all your local branches on your laptop to a
        repository on your desktop machine under `refs/remotes/laptop/`
        hierarchy to back them up, `refs/remotes/laptop/tmp` is removed
        from the remote if you no longer have the branch `tmp` on your
        laptop.


Will queue with a slight fix-ups, including this bit:

> diff --git a/remote.h b/remote.h
> index b395598..341142c 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
>  enum match_refs_flags {
>  	MATCH_REFS_NONE		= 0,
>  	MATCH_REFS_ALL 		= (1 << 0),
> -	MATCH_REFS_MIRROR	= (1 << 1)
> +	MATCH_REFS_MIRROR	= (1 << 1),
> +	MATCH_REFS_PRUNE	= (1 << 2),
>  };

Lose the ',' at the end, for the same reason why deleted line did not have
one.

Thanks.

^ permalink raw reply

* how do I review gitk release notes
From: Neal Kreitzinger @ 2012-02-23  0:47 UTC (permalink / raw)
  To: git

What is the best way to get the corresponding gitk release notes for git 
releases (so I can see the detailed gitk release notes behind the git 
release note "various gitk enhancements and fixes")?

v/r,
neal 

^ permalink raw reply

* Re: [PATCHv3 1/5] refs: add match_pattern()
From: Tom Grennan @ 2012-02-23  0:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, peff, jasampler, pclouds
In-Reply-To: <7v62ey8ktp.fsf@alter.siamese.dyndns.org>

On Wed, Feb 22, 2012 at 04:17:22PM -0800, Junio C Hamano wrote:
>Tom Grennan <tmgrennan@gmail.com> writes:
>
>> Yes, I tried to minimize change but the conversion of argv's to
>> string_list's won't add that much.
>
>How about _not_ using string_list?  After all, string_list is not just a
>collection of strings, but is a table to hold strings with attributes.  I
>thought argv_array is more appropriate abstraction for the purpose of your
>patch.

OK, It looks like I should also add a common parse_opt_argv_array() to
parse-options-cb.  Of course that would be in a separate, dependent
patch.

>>>	ref_match_pattern("refs/heads/master",
>>>        		  ["maste?", NULL],
>>>                          ["refs/heads/", NULL],
>>>                          (REF_MATCH_FNM_PATH|REF_MATCH_LEADING));
>>>
>>>Note that the above "ref_match_pattern()" gives the same "flags" for the
>>>call to match_one() for elements in both positive and negative array and
>>>it is very deliberate.  See review comment to [3/5] for the reasoning.
>>
>> OK, I think that I understand, but please confirm, you'd expect no output in
>> the above example, right?
>
>"maste?" would match with FNM_PATHNAME with "refs/heads/master" but
>the negative "refs/heads/" matches with it, so yeah, I expect that the
>function would return false.

thanks, that's a good test case.

-- 
TomG

^ permalink raw reply

* Re: [PATCHv3 3/5] tag --exclude option
From: Junio C Hamano @ 2012-02-23  1:00 UTC (permalink / raw)
  To: Tom Grennan; +Cc: git, peff, jasampler, pclouds
In-Reply-To: <20120223002215.GE2410@tgrennan-laptop>

Tom Grennan <tmgrennan@gmail.com> writes:

> On Tue, Feb 21, 2012 at 10:33:29PM -0800, Junio C Hamano wrote:
>
>>As you do not specify what kind of pattern matching is done to this
>>exclude pattern, it is important to use the same logic between positive
>>and negative ones to give users a consistent UI.  Unfortunately we use
>>fnmatch without FNM_PATHNAME for positive ones, so this exclude pattern
>>needs to follow the same semantics to reduce confusion.
>>
>>This comment applies to all the patches in this series to add this option
>>to existing commands that take the positive pattern.
>
> OK, should I also describe the --no-exclude option?

If you support it, yes.  What does it do?

--no-exclude::
	Cancels all the `--exclude` options given so far on the command line.

perhaps?

^ permalink raw reply

* Re: [PATCH 4/4] push: add 'prune' option
From: Felipe Contreras @ 2012-02-23  1:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7v1upm8jnq.fsf@alter.siamese.dyndns.org>

On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> +--prune::
>> +     Remove remote branches that don't have a local counterpart. For example
>> +     a remote branch `tmp` will be removed if a local branch with the same
>> +     name doesn't exist any more. This also respects refspecs, e.g.
>> +     `refs/heads/*:refs/tmp/*` would make sure that remote `refs/tmp/foo`
>> +     will be removed if `refs/heads/foo` doesn't exist.
>
> I do not think it adds much useful information to mention `tmp` only once
> over what is already said by the first sentence.  Also, the first sentence
> of the example does not make it clear that it is assuming a same-for-same
> mapping.

Sure, the first sentence doesn't make it clear, but it would be a
valid and obvious assumption. The second sentence makes it clear, and
the name `tmp` immediately evokes a branch that will probably be
removed.

> Coming up with a precise technical description is easy, but it is hard to
> explain to the end user in easy terms, and I commend you for attempting to
> add an example in a short single sentence, though.
>
> Perhaps spelling out the underlying assumption the example makes is the
> best we could do here without going too technical.
>
>        ... For example, if you are pushing all your local branches to
>        update the local branches of the remote,

Yeah, but that's 'git push --all', and that's not the common
operation--'git push' is. So that's what I presumed the reader would
assume, and it really doesn't make a difference as to what will
follow:

>        `tmp` branch will be
>        removed from the remote if you removed your `tmp` branch locally.

This reuses the name `tmp`, which seems to be your objective, but it
doesn't explain _why_ it would remove `tmp`; is it because `tmp` is
the upstream branch, or is it because it has the same name?

>        If you are pushing all your local branches on your laptop to a
>        repository on your desktop machine under `refs/remotes/laptop/`
>        hierarchy to back them up, `refs/remotes/laptop/tmp` is removed
>        from the remote if you no longer have the branch `tmp` on your
>        laptop.

Unfortunately, I as a reader have trouble understanding this. More
specifically I have trouble understanding where `refs/remotes/laptop/`
is coming from, and what it is meaning. I have always pictured
`refs/remotes` as something that 'git fetch' updates, and always from
the relevant repository. While eventually I could understand what this
thing is doing, it took me more than one read, and I had to read
slowly, and even then it seems completely non-standard to me.

I think a synthetic example, like `refs/heads/*:refs/tmp/*`, is much
easier to understand because it doesn't mess up with any established
refs, and also has the advantage that it shows the relevant refspec,
which is useful for people that are not familiar with refspecs, and in
fact, people could try it out without messing with their current refs.

>> diff --git a/remote.h b/remote.h
>> index b395598..341142c 100644
>> --- a/remote.h
>> +++ b/remote.h
>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
>>  enum match_refs_flags {
>>       MATCH_REFS_NONE         = 0,
>>       MATCH_REFS_ALL          = (1 << 0),
>> -     MATCH_REFS_MIRROR       = (1 << 1)
>> +     MATCH_REFS_MIRROR       = (1 << 1),
>> +     MATCH_REFS_PRUNE        = (1 << 2),
>>  };
>
> Lose the ',' at the end, for the same reason why deleted line did not have
> one.

And why is that? Isn't git c99? That comma would only ensure that the
next patch that touches this would be two lines instead of one.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: how do I review gitk release notes
From: Junio C Hamano @ 2012-02-23  1:13 UTC (permalink / raw)
  To: Neal Kreitzinger; +Cc: git
In-Reply-To: <ji42ab$71s$1@dough.gmane.org>

"Neal Kreitzinger" <neal@rsss.com> writes:

> What is the best way to get the corresponding gitk release notes for git 
> releases (so I can see the detailed gitk release notes behind the git 
> release note "various gitk enhancements and fixes")?

Unfortunately, the best way I can think of is that you write it yourself
by looking at the log; I do not think there is any selarate release notes
written by anybody.

$ git log -1 --oneline --first-parent master -- gitk-git

will show that 09bb4eb (Merge git://ozlabs.org/~paulus/gitk, 2011-12-16)
was the last merge, so

$ git log -p 09bb4eb^2
$ git shortlog 09bb4eb^2

would be the "raw" data source to study.

^ permalink raw reply

* Re: [PATCH 4/4] push: add 'prune' option
From: Junio C Hamano @ 2012-02-23  1:31 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Jeff King
In-Reply-To: <CAMP44s1s_NeXeUpxDR0cLoLGjSaf-0E_62MzAxeiHS=-6c045A@mail.gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Yeah, but that's 'git push --all', and that's not the common
> operation--'git push' is.

"git push" is common but that does not give you a solid base to guess what
the reader's assumption would be.  Are you assuming "matching" semantics?

> So that's what I presumed the reader would
> assume,...

I do not want let us guess what the reader assumes, as many people seem to
suggest setting push.default to different values and that would change
what the reader would assume.  That was the whole reason that I suggested
to spell the assumption out, so that the reader's assumption does not have
to get into the picture.

> This reuses the name `tmp`, which seems to be your objective, but it
> doesn't explain _why_ it would remove `tmp`; is it because `tmp` is
> the upstream branch, or is it because it has the same name?

The example is to clarify "local counterpart" in the main text.  I
actually would prefer to get rid of `tmp` but I left it as-is as you
wrote.  The exact name used in the example does not matter, whether it is
`tmp` or `xyzzy`.

> Unfortunately, I as a reader have trouble understanding this. More
> specifically I have trouble understanding where `refs/remotes/laptop/`
> is coming from, and what it is meaning. I have always pictured
> `refs/remotes` as something that 'git fetch' updates, and always from
> the relevant repository.

The layout is the recommended set-up to emulate a fetch with a push in the
reverse direction, which I thought anybody should notice.  It is a failure
in our documentation that even an expert didn't.

>>> diff --git a/remote.h b/remote.h
>>> index b395598..341142c 100644
>>> --- a/remote.h
>>> +++ b/remote.h
>>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
>>>  enum match_refs_flags {
>>>       MATCH_REFS_NONE         = 0,
>>>       MATCH_REFS_ALL          = (1 << 0),
>>> -     MATCH_REFS_MIRROR       = (1 << 1)
>>> +     MATCH_REFS_MIRROR       = (1 << 1),
>>> +     MATCH_REFS_PRUNE        = (1 << 2),
>>>  };
>>
>> Lose the ',' at the end, for the same reason why deleted line did not have
>> one.
>
> And why is that?

Because I told you so ;-).

More seriously, we have had patches to accomodate other people's compilers
by dropping the last comma in enum {}.  See c9b6782 (enums: omit trailing
comma for portability, 2011-03-16), 4b05548 (enums: omit trailing comma
for portability, 2010-05-14) for examples.

^ permalink raw reply

* Re: [PATCH 4/4] push: add 'prune' option
From: Felipe Contreras @ 2012-02-23  2:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vlinu72ui.fsf@alter.siamese.dyndns.org>

On Thu, Feb 23, 2012 at 3:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Yeah, but that's 'git push --all', and that's not the common
>> operation--'git push' is.
>
> "git push" is common but that does not give you a solid base to guess what
> the reader's assumption would be.  Are you assuming "matching" semantics?

But if you want to spell the assumption, why not spell the most common one?

>> So that's what I presumed the reader would
>> assume,...
>
> I do not want let us guess what the reader assumes, as many people seem to
> suggest setting push.default to different values and that would change
> what the reader would assume.

No, it wouldn't. Almost all readers are familiar about what 'git push'
does by default--I would even adventure to say all--the ones that set
a custom push.default would do it because they _know_ they don't want
the default.

And in fact, as I said, it doesn't matter what the reader has set in
push.default, or if the reader is assuming 'git push --all', or 'git
push :', or 'git push --tags', or 'git push refs/heads/*', the end
result is the same.

> That was the whole reason that I suggested
> to spell the assumption out, so that the reader's assumption does not have
> to get into the picture.

I don't think it matters what the reader assumes, because the end
result is the same, so there's no reason to spell any assumption.

>> This reuses the name `tmp`, which seems to be your objective, but it
>> doesn't explain _why_ it would remove `tmp`; is it because `tmp` is
>> the upstream branch, or is it because it has the same name?
>
> The example is to clarify "local counterpart" in the main text.  I
> actually would prefer to get rid of `tmp` but I left it as-is as you
> wrote.  The exact name used in the example does not matter, whether it is
> `tmp` or `xyzzy`.

I believe `tmp` is more useful than `xyzzy`, or even more than
`master`. I already explained why.

>> Unfortunately, I as a reader have trouble understanding this. More
>> specifically I have trouble understanding where `refs/remotes/laptop/`
>> is coming from, and what it is meaning. I have always pictured
>> `refs/remotes` as something that 'git fetch' updates, and always from
>> the relevant repository.
>
> The layout is the recommended set-up to emulate a fetch with a push in the
> reverse direction, which I thought anybody should notice.  It is a failure
> in our documentation that even an expert didn't.

The problem is not figuring out the `refs/remotes/foo`, but picturing
the fact that there are two repositories, the user ran on one of them
'git remote add laptop', and then 'git remote add pc', and then 'git
push --prune refs/heads*:refs/remotes/laptop/*'. Lots of things to
keep in mind for this paragraph. And, as you say, it's the reverse
direction of a fetch, so it's immediately weird.

Moreover, I find it strange that you want to rely on the assumption
that the reader is familiar with the pattern `refs/remotes/foo`, and
the refspec syntax, which is pretty deep plumbing, and not on the
default action of 'git push', which is high-level porcelain, and as I
said, not even needed to understand my example.

>>>> diff --git a/remote.h b/remote.h
>>>> index b395598..341142c 100644
>>>> --- a/remote.h
>>>> +++ b/remote.h
>>>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
>>>>  enum match_refs_flags {
>>>>       MATCH_REFS_NONE         = 0,
>>>>       MATCH_REFS_ALL          = (1 << 0),
>>>> -     MATCH_REFS_MIRROR       = (1 << 1)
>>>> +     MATCH_REFS_MIRROR       = (1 << 1),
>>>> +     MATCH_REFS_PRUNE        = (1 << 2),
>>>>  };
>>>
>>> Lose the ',' at the end, for the same reason why deleted line did not have
>>> one.
>>
>> And why is that?
>
> Because I told you so ;-).

You didn't tell me anything about the previous line :)

> More seriously, we have had patches to accomodate other people's compilers
> by dropping the last comma in enum {}.  See c9b6782 (enums: omit trailing
> comma for portability, 2011-03-16), 4b05548 (enums: omit trailing comma
> for portability, 2010-05-14) for examples.

Yeah, I remembered those patches after sending the mail. Shame. Maybe
next decade =/

Cheers.
-- 
Felipe Contreras

^ permalink raw reply

* What's cooking in git.git (Feb 2012, #08; Wed, 22)
From: Junio C Hamano @ 2012-02-23  2:34 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'.

You can find the changes described here in the integration branches of the
repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

But I seem to be getting this from the github:

  ERROR: Permission to git/git.git denied to gitster.
  fatal: The remote end hung up unexpectedly

so https://github.com/git/git/ is not updated.  All others are OK.

--------------------------------------------------
[New Topics]

* jb/filter-ignore-sigpipe (2012-02-21) 1 commit
 - Ignore SIGPIPE when running a filter driver

Looked sane.
Will merge to "next".

* jc/pickaxe-ignore-case (2012-02-21) 1 commit
 - pickaxe: allow -i to search in patch case-insensitively

* jc/doc-merge-options (2012-02-22) 1 commit
 - Documentation/merge-options.txt: group "ff" related options together

Documentation for "git merge" had "--ff-only" far away from other options
related to the handling of fast-forward merges.

* ph/cherry-pick-advice-refinement (2012-02-22) 1 commit
 - cherry-pick: No advice to commit if --no-commit

* pj/completion-remote-set-url-branches (2012-02-22) 2 commits
 - completion: normalize increment/decrement style
 - completion: remote set-* <name> and <branch>

* th/git-diffall (2012-02-22) 1 commit
 - contrib: added git-diffall

--------------------------------------------------
[Graduated to "master"]

* jk/diff-highlight (2012-02-13) 5 commits
  (merged to 'next' on 2012-02-20 at ba040ae)
 + diff-highlight: document some non-optimal cases
 + diff-highlight: match multi-line hunks
 + diff-highlight: refactor to prepare for multi-line hunks
 + diff-highlight: don't highlight whole lines
 + diff-highlight: make perl strict and warnings fatal

Updates diff-highlight (in contrib/).

* jn/gitweb-unborn-head (2012-02-17) 1 commit
  (merged to 'next' on 2012-02-20 at 80e3ff2)
 + gitweb: Fix "heads" view when there is no current branch

"gitweb" compared non-existent value of HEAD with the names of commit
objects at tips of branches, triggering runtime warnings.

--------------------------------------------------
[Stalled]

* jl/maint-submodule-relative (2012-02-09) 2 commits
 - submodules: always use a relative path from gitdir to work tree
 - submodules: always use a relative path to gitdir

The second one looked iffy.

* hv/submodule-recurse-push (2012-02-13) 3 commits
 - push: teach --recurse-submodules the on-demand option
 - Refactor submodule push check to use string list instead of integer
 - Teach revision walking machinery to walk multiple times sequencially

The bottom one was not clearly explained and needs a reroll.

* zj/diff-stat-dyncol (2012-02-15) 6 commits
 . diff --stat: use less columns for change counts
 - (squash to the previous -- replace the last line of the log with the following)
 - diff --stat: use the full terminal width
 - (squash to the previous -- replace the log message with this)
 - diff --stat: tests for long filenames and big change counts
 - Merge branches zj/decimal-width and zj/term-columns

I am beginning to think that the last one should wait until the dust from
the earlier part settles.

* jc/advise-push-default (2011-12-18) 1 commit
 - push: hint to use push.default=upstream when appropriate

Peff had a good suggestion outlining an updated code structure so that
somebody new can try to dip his or her toes in the development. Any
takers?

* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
 - fixup! 15eaaf4
 - git-svn, perl/Git.pm: extend Git::prompt helper for querying users
 - perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS

The bottom one has been replaced with a rewrite based on comments from
Ævar. The second one needs more work, both in perl/Git.pm and prompt.c, to
give precedence to tty over SSH_ASKPASS when terminal is available.

* jc/split-blob (2012-01-24) 6 commits
 - chunked-object: streaming checkout
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - varint-in-pack: refactor varint encoding/decoding

Not ready.

I finished the streaming checkout codepath, but as explained in 127b177
(bulk-checkin: support chunked-object encoding, 2011-11-30), these are
still early steps of a long and painful journey. At least pack-objects and
fsck need to learn the new encoding for the series to be usable locally,
and then index-pack/unpack-objects needs to learn it to be used remotely.

Given that I heard a lot of noise that people want large files, and that I
was asked by somebody at GitTogether'11 privately for an advice on how to
pay developers (not me) to help adding necessary support, I am somewhat
dissapointed that the original patch series that was sent almost two
months ago still remains here without much comments and updates from the
developer community. I even made the interface to the logic that decides
where to split chunks easily replaceable, and I deliberately made the
logic in the original patch extremely stupid to entice others, especially
the "bup" fanboys, to come up with a better logic, thinking that giving
people an easy target to shoot for, they may be encouraged to help
out. The plan is not working :-(.

* nd/columns (2012-02-08) 15 commits
 . column: Fix some compiler and sparse warnings
 . column: add a corner-case test to t3200
 . columns: minimum coding style fixes
 . tag: add --column
 . column: support piping stdout to external git-column process
 . status: add --column
 . branch: add --column
 . help: reuse print_columns() for help -a
 . column: add column.ui for default column output settings
 . column: support columns with different widths
 . column: add columnar layout
 . Stop starting pager recursively
 . Add git-column and column mode parsing
 . column: add API to print items in columns
 . Save terminal width before setting up pager

Expecting a reroll on top of zj/term-columns topic.

--------------------------------------------------
[Cooking]

* fc/push-prune (2012-02-22) 4 commits
 - push: add '--prune' option
 - remote: refactor code into alloc_delete_ref()
 - remote: reorganize check_pattern_match()
 - remote: use a local variable in match_push_refs()

Rerolled and looked sane.
Will merge to "next".

* jc/add-refresh-unmerged (2012-02-17) 1 commit
  (merged to 'next' on 2012-02-21 at 09f8721)
 + refresh_index: do not show unmerged path that is outside pathspec

"git add --refresh <pathspec>" warned about unmerged paths outside the
given pathspec.

* jc/diff-ignore-case (2012-02-19) 6 commits
 - diff -i
 - diff: --ignore-case
 - xdiff: introduce XDF_IGNORE_CASE
 - xdiff: introduce XDF_INEXACT_MATCH
 - xdiff: PATIENCE/HISTOGRAM are not independent option bits
 - xdiff: remove XDL_PATCH_* macros

"git diff" learns "--ignore-case" option.

* jn/gitweb-hilite-regions (2012-02-19) 5 commits
 - gitweb: Use esc_html_match_hl() in 'grep' search
 - gitweb: Highlight matched part of shortened project description
 - gitweb: Highlight matched part of project description when searching projects
 - gitweb: Highlight matched part of project name when searching projects
 - gitweb: Introduce esc_html_hl_regions
 (this branch uses jn/gitweb-search-optim.)

Not reviewed and do not know what this is about yet ;-).

* jn/gitweb-search-optim (2012-02-19) 3 commits
 - gitweb: Faster project search
 - gitweb: Option for filling only specified info in fill_project_list_info
 - gitweb: Refactor checking if part of project info need filling
 (this branch is used by jn/gitweb-hilite-regions.)

The API introduced in the second step still has yucky design, but at least
it is more clear than the previous rounds what this is trying to do.

* js/configure-libintl (2012-02-20) 1 commit
  (merged to 'next' on 2012-02-21 at 79d7ccc)
 + configure: don't use -lintl when there is no gettext support

Build fix for autoconf, meant for 'maint' track.

* pj/remote-set-branches-usage-fix (2012-02-19) 1 commit
  (merged to 'next' on 2012-02-21 at cb71d0e)
 + remote: fix set-branches usage and documentation

Documentation fix.

* tr/perftest (2012-02-17) 3 commits
  (merged to 'next' on 2012-02-20 at 4c75ba9)
 + Add a performance test for git-grep
 + Introduce a performance testing framework
 + Move the user-facing test library to test-lib-functions.sh

* jb/required-filter (2012-02-17) 1 commit
 - Add a setting to require a filter to be successful

A content filter used to be a way to make the recorded contents "more
useful", but this defines a way to optionally mark a filter "required".
Will merge to "next" after waiting for a few more days for comments.

* jk/config-include (2012-02-17) 10 commits
  (merged to 'next' on 2012-02-20 at 7b150b7)
 + config: add include directive
 + config: eliminate config_exclusive_filename
 + config: stop using config_exclusive_filename
 + config: provide a version of git_config with more options
 + config: teach git_config_rename_section a file argument
 + config: teach git_config_set_multivar_in_file a default path
 + config: copy the return value of prefix_filename
 + t1300: add missing &&-chaining
 + docs/api-config: minor clarifications
 + docs: add a basic description of the config API

An assignment to the include.path pseudo-variable causes the named file
to be included in-place when Git looks up configuration variables.

Reverted the earlier round from 'next' and then fixed up further.

* ld/git-p4-expanded-keywords (2012-02-14) 1 commit
  (merged to 'next' on 2012-02-16 at a9004c5)
 + git-p4: add initial support for RCS keywords

Teach git-p4 to unexpand $RCS$-like keywords that are embedded in
tracked contents in order to reduce unnecessary merge conflicts.

Waiting for follow-up fix-up patches.

^ permalink raw reply

* [ANNOUNCE] Git 1.7.9.2
From: Junio C Hamano @ 2012-02-23  2:38 UTC (permalink / raw)
  To: git

The latest maintenance release Git 1.7.9.2 is now available at the
usual places.

The release tarballs are found at:

    http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

7aff1048480a8637de94e8d82744d312c0b5e060  git-1.7.9.2.tar.gz
3cf13b03b2f64d0458212232cc18983231f8251e  git-htmldocs-1.7.9.2.tar.gz
d6992d899fb70e40983f94a2f96ad24b8ee93557  git-manpages-1.7.9.2.tar.gz

Also the following public repositories all have a copy of the v1.7.9.2
tag and the maint branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.7.9.2 Release Notes
==========================

Fixes since v1.7.9.1
--------------------

 * Bash completion script (in contrib/) did not like a pattern that
   begins with a dash to be passed to __git_ps1 helper function.

 * Adaptation of the bash completion script (in contrib/) for zsh
   incorrectly listed all subcommands when "git <TAB><TAB>" was given
   to ask for list of porcelain subcommands.

 * The build procedure for profile-directed optimized binary was not
   working very well.

 * Some systems need to explicitly link -lcharset to get locale_charset().

 * t5541 ignored user-supplied port number used for HTTP server testing.

 * The error message emitted when we see an empty loose object was
   not phrased correctly.

 * The code to ask for password did not fall back to the terminal
   input when GIT_ASKPASS is set but does not work (e.g. lack of X
   with GUI askpass helper).

 * We failed to give the true terminal width to any subcommand when
   they are invoked with the pager, i.e. "git -p cmd".

 * map_user() was not rewriting its output correctly, which resulted
   in the user visible symptom that "git blame -e" sometimes showed
   excess '>' at the end of email addresses.

 * "git checkout -b" did not allow switching out of an unborn branch.

 * When you have both .../foo and .../foo.git, "git clone .../foo" did not
   favor the former but the latter.

 * "git commit" refused to create a commit when entries added with
   "add -N" remained in the index, without telling Git what their content
   in the next commit should be. We should have created the commit without
   these paths.

 * "git diff --stat" said "files", "insertions", and "deletions" even
   when it is showing one "file", one "insertion" or one "deletion".

 * The output from "git diff --stat" for two paths that have the same
   amount of changes showed graph bars of different length due to the
   way we handled rounding errors.

 * "git grep" did not pay attention to -diff (hence -binary) attribute.

 * The transport programs (fetch, push, clone)ignored --no-progress
   and showed progress when sending their output to a terminal.

 * Sometimes error status detected by a check in an earlier phase of
   "git receive-pack" (the other end of "git push") was lost by later
   checks, resulting in false indication of success.

 * "git rev-list --verify" sometimes skipped verification depending on
   the phase of the moon, which dates back to 1.7.8.x series.

 * Search box in "gitweb" did not accept non-ASCII characters correctly.

 * Search interface of "gitweb" did not show multiple matches in the same file
   correctly.

Also contains minor fixes and documentation updates.

----------------------------------------------------------------

Changes since v1.7.9.1 are as follows:

Christian Hammerl (1):
      completion: Allow dash as the first character for __git_ps1

Clemens Buchacher (4):
      git rev-list: fix invalid typecast
      push/fetch/clone --no-progress suppresses progress output
      t5541: check error message against the real port number used
      do not override receive-pack errors

Felipe Contreras (3):
      completion: work around zsh option propagation bug
      completion: use ls -1 instead of rolling a loop to do that ourselves
      completion: simplify __gitcomp and __gitcomp_nl implementations

Jakub Narebski (2):
      gitweb: Allow UTF-8 encoded CGI query parameters and path_info
      gitweb: Fix 'grep' search for multiple matches in file

Jeff King (12):
      grep: make locking flag global
      grep: move sha1-reading mutex into low-level code
      grep: refactor the concept of "grep source" into an object
      convert git-grep to use grep_source interface
      grep: drop grep_buffer's "name" parameter
      grep: cache userdiff_driver in grep_source
      grep: respect diff attributes for binary-ness
      grep: load file data after checking binary-ness
      grep: pre-load userdiff drivers when threaded
      standardize and improve lookup rules for external local repos
      prompt: clean up strbuf usage
      prompt: fall back to terminal if askpass fails

Jiang Xin (2):
      i18n: git-commit whence_s "merge/cherry-pick" message
      i18n: format_tracking_info "Your branch is behind" message

Johannes Sixt (1):
      Makefile: fix syntax for older make

Junio C Hamano (8):
      mailmap: always return a plain mail address from map_user()
      git checkout -b: allow switching out of an unborn branch
      commit: ignore intent-to-add entries instead of refusing
      diff --stat: show bars of same length for paths with same amount of changes
      Update draft release notes to 1.7.9.2
      Update draft release notes to 1.7.9.2
      Update draft release notes to 1.7.9.2
      Git 1.7.9.2

Matthieu Moy (1):
      fsck: give accurate error message on empty loose object files

Namhyung Kim (2):
      ctype.c only wants git-compat-util.h
      ctype: implement islower/isupper macro

Nguyễn Thái Ngọc Duy (3):
      sha1_file.c: move the core logic of find_pack_entry() into fill_pack_entry()
      find_pack_entry(): do not keep packed_git pointer locally
      Use correct grammar in diffstat summary line

Philip Jägenstedt (2):
      completion: remove stale "to submit patches" documentation
      completion: use tabs for indentation

Ralf Thielow (2):
      completion: --edit-description option for git-branch
      completion: --list option for git-branch

Theodore Ts'o (1):
      Fix build problems related to profile-directed optimization

Zbigniew Jędrzejewski-Szmek (2):
      pager: find out the terminal width before spawning the pager
      man: rearrange git synopsis to fit in 80 lines

Дилян Палаузов (1):
      Makefile: introduce CHARSET_LIB to link with -lcharset

^ permalink raw reply

* Re: [PATCH 2/2] bundle: use a strbuf to scan the log for boundary commits
From: Junio C Hamano @ 2012-02-23  3:20 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Jannis Pohlmann
In-Reply-To: <fa1553d59714fd89fdab1bf54af19ac631a30a8c.1329939233.git.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> +# If "ridiculous" is at least 1004 chars, this traps a bug in old
> +# versions where the resulting 1025-char line (with --pretty=oneline)
> +# was longer than a 1024-char buffer
> +test_expect_success 'ridiculously long subject in boundary' '
> +
> +	: > file4 &&
> +	test_tick &&
> +	git add file4 &&
> +	printf "abcdefghijkl %s\n" $(seq 1 100) | git commit -F - &&
> +	test_commit fifth &&
> +	git bundle create long-subject-bundle.bdl HEAD^..HEAD &&
> +	git fetch long-subject-bundle.bdl &&
> +	sed -n "/^-/{p;q}" long-subject-bundle.bdl > boundary &&
> +	grep "^-$_x40 " boundary
> +
> +'

Hmm, aside from the use of seq J6t mentioned, I am not sure what this is
testing.

I thought the point is to make "--oneline" output produce a long line
whose 1024th character is '-' and make the fgets() based parser to mistake
it as the beginning of a line that records a boundary commit, but you do
not seem to have any '-' there.

^ permalink raw reply

* Re: [PATCH 2/2] bundle: use a strbuf to scan the log for boundary commits
From: Junio C Hamano @ 2012-02-23  3:38 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Jannis Pohlmann
In-Reply-To: <fa1553d59714fd89fdab1bf54af19ac631a30a8c.1329939233.git.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> diff --git a/bundle.c b/bundle.c
> index 313de42..0dbd174 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -234,7 +234,7 @@ int create_bundle(struct bundle_header *header, const char *path,
>  	const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));
>  	const char **argv_pack = xmalloc(6 * sizeof(const char *));
>  	int i, ref_count = 0;
> -	char buffer[1024];
> +	struct strbuf buf = STRBUF_INIT;
>  	struct rev_info revs;
>  	struct child_process rls;
>  	FILE *rls_fout;
> @@ -266,16 +266,16 @@ int create_bundle(struct bundle_header *header, const char *path,
>  	if (start_command(&rls))
>  		return -1;
>  	rls_fout = xfdopen(rls.out, "r");
> -	while (fgets(buffer, sizeof(buffer), rls_fout)) {
> +	while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {

I'd add strbuf_release(&buf) after this loop.

Perhaps we would want to squash something like this to the test to avoid
"seq", using J6t's idea.  The issue is that we do not write the end of
line for the long boundary (because it is hidden from us), keep reading
and rejecting bogus letters, and then the tip is written without
terminating the boundary line.  So checking boundary line is not a very
useful test, but "fetch" and "list-heads" are.

 t/t5704-bundle.sh |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/t/t5704-bundle.sh b/t/t5704-bundle.sh
index 7c2f307..5319b84 100755
--- a/t/t5704-bundle.sh
+++ b/t/t5704-bundle.sh
@@ -59,19 +59,20 @@ test_expect_success 'empty bundle file is rejected' '
 
 '
 
-# If "ridiculous" is at least 1004 chars, this traps a bug in old
-# versions where the resulting 1025-char line (with --pretty=oneline)
-# was longer than a 1024-char buffer
+# This triggers a bug in older versions where the resulting line (with
+# --pretty=oneline) was longer than a 1024-char buffer.
 test_expect_success 'ridiculously long subject in boundary' '
 
-	: > file4 &&
+	: >file4 &&
 	test_tick &&
 	git add file4 &&
-	printf "abcdefghijkl %s\n" $(seq 1 100) | git commit -F - &&
+	printf "%0982d\n" 0 | git commit -F - &&
 	test_commit fifth &&
 	git bundle create long-subject-bundle.bdl HEAD^..HEAD &&
+	git bundle list-heads long-subject-bundle.bdl >heads &&
+	test -s heads &&
 	git fetch long-subject-bundle.bdl &&
-	sed -n "/^-/{p;q}" long-subject-bundle.bdl > boundary &&
+	sed -n "/^-/{p;q}" long-subject-bundle.bdl >boundary &&
 	grep "^-$_x40 " boundary
 
 '
-- 
1.7.9.2.258.g4407a

^ permalink raw reply related

* Re: [PATCH 0/8 v6] diff --stat: use the full terminal width
From: Junio C Hamano @ 2012-02-23  5:08 UTC (permalink / raw)
  To: Zbigniew Jędrzejewski-Szmek; +Cc: git, Michael J Gruber, pclouds, j.sixt
In-Reply-To: <1329775034-21551-1-git-send-email-zbyszek@in.waw.pl>

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:

> this is v6, with a new approach.

When you reroll, please make sure you do not run format-patch with
nonstandard settings, e.g. 

	diff --git t/t4052-stat-output.sh t/t4052-stat-output.sh

^ permalink raw reply

* Re: [PATCH 0/8 v6] diff --stat: use the full terminal width
From: Miles Bader @ 2012-02-23  7:29 UTC (permalink / raw)
  To: Zbigniew Jędrzejewski-Szmek
  Cc: Nguyen Thai Ngoc Duy, git, gitster, Michael J Gruber, j.sixt
In-Reply-To: <4F43C23C.8070304@in.waw.pl>

Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:
> I use (or would like to be able to use) the --stat output to select with
> the mouse and paste into something like
>   emacsclient -n <pathname>
> This would be harder with the grouping, because I'd have to select two
> parts and paste two times and type a slash. So for me this would be a
> minus.

Of course, git's diffstats often already violate that property (e.g.,
abbreviations and the renaming syntax)...  In cases where such a
grouping syntax avoids information-losing abbreviations, it might even
be an improvement over the current situation (even if the info was
split up, at least it would be there).

If the heuristics were tuned so that the syntax only kicked in for
cases where deep nesting or long names would otherwise make
abbreviations likely, maybe it would be a general improvement.

However, it does seem like the additional inconsistency of
presentation might make diffstats harder to read or more ugly in some
cases...

-miles

-- 
Dawn, n. When men of reason go to bed.

^ permalink raw reply

* [PATCHv5] git-p4: RCS keyword handling
From: Luke Diamand @ 2012-02-23  7:51 UTC (permalink / raw)
  To: git; +Cc: Pete Wyckoff, Eric Scouten, Junio C Hamano, Luke Diamand

Updated RCS keyword fix for git-p4, incorporating fixes to test cases
missed earlier.

Luke Diamand (1):
  git-p4: add initial support for RCS keywords

 Documentation/git-p4.txt   |    5 +
 contrib/fast-import/git-p4 |  118 ++++++++++++--
 t/t9810-git-p4-rcs.sh      |  388 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 501 insertions(+), 10 deletions(-)
 create mode 100755 t/t9810-git-p4-rcs.sh

-- 
1.7.9.259.ga92e

^ 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