Git development
 help / color / mirror / Atom feed
* [PATCH] Documentation/show-ref: correct order of --heads and --tags
From: pangyanhan @ 2011-08-26  2:26 UTC (permalink / raw)
  To: git; +Cc: Pang Yan Han

From: Pang Yan Han <pangyanhan@gmail.com>

Signed-off-by: Pang Yan Han <pangyanhan@gmail.com>
---
 Documentation/git-show-ref.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
index 3c45895..5a646e4 100644
--- a/Documentation/git-show-ref.txt
+++ b/Documentation/git-show-ref.txt
@@ -34,8 +34,8 @@ OPTIONS
 
 	Show the HEAD reference.
 
---tags::
 --heads::
+--tags::
 
 	Limit to only "refs/heads" and "refs/tags", respectively.  These
 	options are not mutually exclusive; when given both, references stored
-- 
1.7.6

^ permalink raw reply related

* Re: [PATCH 1/2] revision: keep track of the end-user input from the command line
From: Junio C Hamano @ 2011-08-26  2:21 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git, Brad King, Heiko Voigt
In-Reply-To: <CAGdFq_he-skXpEy903RSY2ravDXo1eVCN19RzJeHXQCT__HTmA@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

>> Prepare a separate array to keep track of what syntactic element was used
>> to cause each object to appear in the pending array from the command line,
>> and populate it as setup_revisions() parses the command line.
>
> Thank you! I was really dreading looking into this myself, so I'm very
> glad that you could find the time to look into it yourself.

I debated long and hard if I should instead fatten object array entry and
shove this information there without adding a new structure, which would
have resulted in something very similar to what you had, so you should
take some credit for the code, and also credit for a large part of the
motivation (the second paragraph in the log is entirely your use case).

We might end up unifying this command line information array and the
pending object array after reviewing what other future callers would want
from this new information, but at least by doing it this way I can rest
assured that no existing code that is unaware of the mechanism would get
any unintended side effects in the earlier rounds.

>> @@ -835,6 +853,7 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
>>                struct object *o = parse_object(sha1);
>>                if (o) {
>>                        o->flags |= cb->all_flags;
>> +                       /* ??? CMDLINEFLAGS ??? */
>>                        add_pending_object(cb->all_revs, o, "");
>>                }
>>                else if (!cb->warned_bad_reflog) {
>
> What is happening here?

We could have add_rev_cmdline() call there if we really wanted to, but I
decided not to do so for two reasons:

(1) with "rev-list -g HEAD", the user is not explicitly mentioning all
    objects in the reflog of the HEAD---it might still make sense to mark
    HEAD as explicitly mentioned as positive, but the code to do so will
    not sit here anyway; and more importantly,

(2) I was appalled by how broken the design and the implementation of
    walking the reflog was (it shoves _all_ the objects in the entire
    history recorded in the reflog to the pending queue before letting the
    caller do an iota of work). It is against the general design of Git,
    and the design of the revision walk machinery in particular that tries
    very hard to be incremental. It goes against a good software design
    taste.

For the latter reason, I think in the longer term we should correct the
implementation to walk the reflog to keep an iterator (a structure that
holds an open file descriptor to a reflog file, and perhaps a little bit
of buffer) in struct rev_info, and teach get_revisions() to lazily read
from there, reading the file backward. Once that happens, this callback
from for_each_reflog_ent() will go away, so I didn't bother.

^ permalink raw reply

* Adding hooks.envelopesendername to the post-receive-email hook script
From: László Monda @ 2011-08-26  1:15 UTC (permalink / raw)
  To: git

Hi List,

I wonder whether anybody thinks that being able to specify
hooks.envelopesendername to set the name of the sender (like what the
sendmail -F option does) would be a useful feature.  If so, I'm also
interested that whoever is in charge is willing to merge such a patch.

Thanks in advance.

-- 
László Monda <http://monda.hu>

^ permalink raw reply

* Re: [PATCH 1/2] revision: keep track of the end-user input from the command line
From: Sverre Rabbelier @ 2011-08-26  1:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Brad King, Heiko Voigt
In-Reply-To: <7vbovdgf73.fsf_-_@alter.siamese.dyndns.org>

Heya,

On Thu, Aug 25, 2011 at 18:00, Junio C Hamano <gitster@pobox.com> wrote:
> Prepare a separate array to keep track of what syntactic element was used
> to cause each object to appear in the pending array from the command line,
> and populate it as setup_revisions() parses the command line.

Thank you! I was really dreading looking into this myself, so I'm very
glad that you could find the time to look into it yourself.

> @@ -835,6 +853,7 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
>                struct object *o = parse_object(sha1);
>                if (o) {
>                        o->flags |= cb->all_flags;
> +                       /* ??? CMDLINEFLAGS ??? */
>                        add_pending_object(cb->all_revs, o, "");
>                }
>                else if (!cb->warned_bad_reflog) {

What is happening here?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH 2/2] revision: do not include sibling history in --ancestry-path output
From: Junio C Hamano @ 2011-08-26  1:00 UTC (permalink / raw)
  To: git; +Cc: Brad King, Heiko Voigt
In-Reply-To: <438ea0b254ccafb3fc9f3431f8f86007cc03132b.1314290439.git.brad.king@kitware.com>

If the commit specified as the bottom of the commit range has a direct
parent that has another child commit that contributed to the resulting
history, "rev-list --ancestry-path" was confused and listed that side
history as well.

             D---E
            /     \
        ---X---A---B---C

In this history, "rev-list --ancestry-path A..C" should list among what
the corresponding command without --ancestry-path option would produce,
namely, D, E, B and C, but limiting the result to those that are
descendant of A (i.e. B and C). Due to the command line parser subtlety
corrected by the previous commit, it also listed those that are descendant
of X as well.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * And this should fix the breakage you demonstrated.

 revision.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/revision.c b/revision.c
index 3e87c86..48a2db4 100644
--- a/revision.c
+++ b/revision.c
@@ -724,12 +724,16 @@ static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li
  * to filter the result of "A..B" further to the ones that can actually
  * reach A.
  */
-static struct commit_list *collect_bottom_commits(struct commit_list *list)
+static struct commit_list *collect_bottom_commits(struct rev_info *revs)
 {
-	struct commit_list *elem, *bottom = NULL;
-	for (elem = list; elem; elem = elem->next)
-		if (elem->item->object.flags & UNINTERESTING)
-			commit_list_insert(elem->item, &bottom);
+	struct commit_list *bottom = NULL;
+	int i;
+	for (i = 0; i < revs->cmdline.nr; i++) {
+		struct rev_cmdline_entry *elem = &revs->cmdline.rev[i];
+		if ((elem->flags & UNINTERESTING) &&
+		    elem->item->type == OBJ_COMMIT)
+			commit_list_insert((struct commit *)elem->item, &bottom);
+	}
 	return bottom;
 }
 
@@ -743,7 +747,7 @@ static int limit_list(struct rev_info *revs)
 	struct commit_list *bottom = NULL;
 
 	if (revs->ancestry_path) {
-		bottom = collect_bottom_commits(list);
+		bottom = collect_bottom_commits(revs);
 		if (!bottom)
 			die("--ancestry-path given but there are no bottom commits");
 	}
-- 
1.7.6.1.385.gb7fcd0

^ permalink raw reply related

* [PATCH 1/2] revision: keep track of the end-user input from the command line
From: Junio C Hamano @ 2011-08-26  1:00 UTC (permalink / raw)
  To: git; +Cc: Brad King, Heiko Voigt, Sverre Rabbelier
In-Reply-To: <438ea0b254ccafb3fc9f3431f8f86007cc03132b.1314290439.git.brad.king@kitware.com>

Given a complex set of revision specifiers on the command line, it is too
late to look at the flags of the objects in the initial traversal list at
the beginning of limit_list() in order to determine what the objects the
end-user explicitly listed on the command line were. The process to move
objects from the pending array to the traversal list may have marked
objects that are not mentioned as UNINTERESTING, when handle_commit()
marked the parents of UNINTERESTING commits mentioned on the command line
by calling mark_parents_uninteresting().

This made "rev-list --ancestry-path ^A ..." to mistakenly list commits
that are descendants of A's parents but that are not descendants of A
itself, as ^A from the command line causes A and its parents marked as
UNINTERESTING before coming to limit_list(), and we try to enumerate the
commits that are descendants of these commits that are UNINTERESTING
before we start walking the history.

It actually is too late even if we inspected the pending object array
before calling prepare_revision_walk(), as some of the same objects might
have been mentioned twice, once as positive and another time as negative.
The "rev-list --some-option A --not --all" command may want to notice,
even if the resulting set is empty, that the user showed some interest in
"A" and do something special about it.

Prepare a separate array to keep track of what syntactic element was used
to cause each object to appear in the pending array from the command line,
and populate it as setup_revisions() parses the command line.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Should apply cleanly on top of your test case.

 revision.c |   37 +++++++++++++++++++++++++++++++++----
 revision.h |   20 ++++++++++++++++++++
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/revision.c b/revision.c
index 96d7fa7..3e87c86 100644
--- a/revision.c
+++ b/revision.c
@@ -797,6 +797,23 @@ static int limit_list(struct rev_info *revs)
 	return 0;
 }
 
+static void add_rev_cmdline(struct rev_info *revs,
+			    struct object *item,
+			    const char *name,
+			    int whence,
+			    unsigned flags)
+{
+	struct rev_cmdline_info *info = &revs->cmdline;
+	int nr = info->nr;
+
+	ALLOC_GROW(info->rev, nr + 1, info->alloc);
+	info->rev[nr].item = item;
+	info->rev[nr].name = name;
+	info->rev[nr].whence = whence;
+	info->rev[nr].flags = flags;
+	info->nr++;
+}
+
 struct all_refs_cb {
 	int all_flags;
 	int warned_bad_reflog;
@@ -809,6 +826,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1, int flag,
 	struct all_refs_cb *cb = cb_data;
 	struct object *object = get_reference(cb->all_revs, path, sha1,
 					      cb->all_flags);
+	add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags);
 	add_pending_object(cb->all_revs, object, path);
 	return 0;
 }
@@ -835,6 +853,7 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 		struct object *o = parse_object(sha1);
 		if (o) {
 			o->flags |= cb->all_flags;
+			/* ??? CMDLINEFLAGS ??? */
 			add_pending_object(cb->all_revs, o, "");
 		}
 		else if (!cb->warned_bad_reflog) {
@@ -871,12 +890,13 @@ static void handle_reflog(struct rev_info *revs, unsigned flags)
 	for_each_reflog(handle_one_reflog, &cb);
 }
 
-static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
+static int add_parents_only(struct rev_info *revs, const char *arg_, int flags)
 {
 	unsigned char sha1[20];
 	struct object *it;
 	struct commit *commit;
 	struct commit_list *parents;
+	const char *arg = arg_;
 
 	if (*arg == '^') {
 		flags ^= UNINTERESTING;
@@ -898,6 +918,7 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
 	for (parents = commit->parents; parents; parents = parents->next) {
 		it = &parents->item->object;
 		it->flags |= flags;
+		add_rev_cmdline(revs, it, arg_, REV_CMD_PARENTS_ONLY, flags);
 		add_pending_object(revs, it, arg);
 	}
 	return 1;
@@ -987,7 +1008,7 @@ static void prepare_show_merge(struct rev_info *revs)
 	revs->limited = 1;
 }
 
-int handle_revision_arg(const char *arg, struct rev_info *revs,
+int handle_revision_arg(const char *arg_, struct rev_info *revs,
 			int flags,
 			int cant_be_filename)
 {
@@ -996,6 +1017,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	struct object *object;
 	unsigned char sha1[20];
 	int local_flags;
+	const char *arg = arg_;
 
 	dotdot = strstr(arg, "..");
 	if (dotdot) {
@@ -1004,6 +1026,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		const char *this = arg;
 		int symmetric = *next == '.';
 		unsigned int flags_exclude = flags ^ UNINTERESTING;
+		unsigned int a_flags;
 
 		*dotdot = 0;
 		next += symmetric;
@@ -1036,10 +1059,15 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 				add_pending_commit_list(revs, exclude,
 							flags_exclude);
 				free_commit_list(exclude);
-				a->object.flags |= flags | SYMMETRIC_LEFT;
+				a_flags = flags | SYMMETRIC_LEFT;
 			} else
-				a->object.flags |= flags_exclude;
+				a_flags = flags_exclude;
+			a->object.flags |= a_flags;
 			b->object.flags |= flags;
+			add_rev_cmdline(revs, &a->object, this,
+					REV_CMD_LEFT, a_flags);
+			add_rev_cmdline(revs, &b->object, next,
+					REV_CMD_RIGHT, flags);
 			add_pending_object(revs, &a->object, this);
 			add_pending_object(revs, &b->object, next);
 			return 0;
@@ -1070,6 +1098,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
 	object = get_reference(revs, arg, sha1, flags ^ local_flags);
+	add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags);
 	add_pending_object_with_mode(revs, object, arg, mode);
 	return 0;
 }
diff --git a/revision.h b/revision.h
index 855464f..031cc7c 100644
--- a/revision.h
+++ b/revision.h
@@ -23,6 +23,23 @@ struct rev_info;
 struct log_info;
 struct string_list;
 
+struct rev_cmdline_info {
+	unsigned int nr;
+	unsigned int alloc;
+	struct rev_cmdline_entry {
+		struct object *item;
+		const char *name;
+		enum {
+			REV_CMD_REF,
+			REV_CMD_PARENTS_ONLY,
+			REV_CMD_LEFT,
+			REV_CMD_RIGHT,
+			REV_CMD_REV
+		} whence;
+		unsigned flags;
+	} *rev;
+};
+
 struct rev_info {
 	/* Starting list */
 	struct commit_list *commits;
@@ -31,6 +48,9 @@ struct rev_info {
 	/* Parents of shown commits */
 	struct object_array boundary_commits;
 
+	/* The end-points specified by the end user */
+	struct rev_cmdline_info cmdline;
+
 	/* Basic information */
 	const char *prefix;
 	const char *def;
-- 
1.7.6.1.385.gb7fcd0

^ permalink raw reply related

* Re: [PATCH] rev-list: Demonstrate breakage with --ancestry-path --all
From: Junio C Hamano @ 2011-08-25 23:49 UTC (permalink / raw)
  To: Brad King; +Cc: git, Heiko Voigt
In-Reply-To: <7v4o15hyxx.fsf@alter.siamese.dyndns.org>

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

> Brad King <brad.king@kitware.com> writes:
>
>> The option added by commit ebdc94f3 (revision: --ancestry-path,
>> 2010-04-20) does not work properly in combination with --all, at least
>> in the case of a criss-cross merge:
>>
>>     b---bc
>>    / \ /
>>   a   X
>>    \ / \
>>     c---cb
>
> Hmm, what should --ancestry-path do given more than one positive commit to
> begin with, let alone --all?

I actually think that this does not have much to do with "criss-cross"-ness. 
Instead of computing those that can be reached from cb, we are computing
those that can be reached from either b, c or cb.

This needs fixing, but it takes a bit more than a quick hack. Stay tuned
;-)

Thanks.

^ permalink raw reply

* Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Eric Sunshine @ 2011-08-25 23:44 UTC (permalink / raw)
  To: Jeff King; +Cc: Boaz Harrosh, git
In-Reply-To: <20110825210654.GA11077@sigill.intra.peff.net>

On 08/25/2011 05:06 PM, Jeff King wrote:
> On Thu, Aug 25, 2011 at 05:00:51PM -0400, Eric Sunshine wrote:
>
>>> Also, any other extensions that would go into such a list? I have no
>>> idea what the common extension is for something like pascal or csharp.
>>
>> C# uses extension ".cs".
>>
>> ".cpp" is common, in fact often required, by Windows compilers.
>
> Thanks, added both to my list.

To clarify, I meant to say that for C++, .cpp is common/required on Windows.

>> What about ".h" and ".hpp"?
>
> How well do our cpp patterns do with header files? I imagine they're
> better than the default, but I don't think I've ever really tried
> anything tricky.

I scanned through a number of revisions for one of my long-running C++ 
projects comparing the diff of header files with and without "*.h 
diff=cpp". In some header files in this project, the oft-used C++ 
keywords public:, protected:, and private: appear at start-of-line. In 
such cases, the default diff emits a less-than-useful hunk header:

     @@ -19,8 +19,8 @@ public:

whereas, "diff=cpp" emits:

     @@ -19,8 +19,8 @@ class Foobar

-- ES

^ permalink raw reply

* Re: [PATCH] allow multiple calls to submodule merge search for the same path
From: Junio C Hamano @ 2011-08-25 23:39 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: Brad King, git
In-Reply-To: <7vsjopgjfs.fsf@alter.siamese.dyndns.org>

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Heiko Voigt <hvoigt@hvoigt.net> writes:
>>
>>> diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
>>> index 8f6f2d6..603fb72 100755
>>> --- a/t/t7405-submodule-merge.sh
>>> +++ b/t/t7405-submodule-merge.sh
>>> @@ -269,7 +269,7 @@ test_expect_success 'setup for recursive merge with submodule' '
>>>  '
>>>  
>>>  # merge should leave submodule unmerged in index
>>> -test_expect_failure 'recursive merge with submodule' '
>>> +test_expect_success 'recursive merge with submodule' '
>>>  	(cd merge-recursive &&
>>>  	 test_must_fail git merge top-bc &&
>>>  	 echo "160000 $(git rev-parse top-cb:sub) 2	sub" > expect2 &&
>>
>> What is this patch based on?
>
> Ah, nevermind. I figured it out. Thanks.

Just FYI; squashed this on top to fix compilation breakage.
Thanks.

 submodule.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/submodule.c b/submodule.c
index 21a57d2..752cd8a 100644
--- a/submodule.c
+++ b/submodule.c
@@ -738,6 +738,7 @@ static int find_first_merges(struct object_array *result, const char *path,
 	struct object_array merges;
 	struct commit *commit;
 	int contains_another;
+	FILE *out;
 
 	char merged_revision[42];
 	const char *rev_args[] = { "rev-list", "--merges", "--ancestry-path",
@@ -762,14 +763,15 @@ static int find_first_merges(struct object_array *result, const char *path,
 	if (start_command(&cp))
 		die("Could not run 'git rev-list --merges --ancestry-path --all %s' "
 				"command in submodule %s", merged_revision, path);
-	FILE *out = fdopen(cp.out, "r");
+	out = fdopen(cp.out, "r");
 	if (!out)
 		die("Could not open pipe of rev-list command.");
 
 	/* save all revisions from the above list that contain b */
 	while (strbuf_getline(&one_rev, out, '\n') != EOF) {
+		struct object *o;
 		commit = lookup_commit_reference_by_name(one_rev.buf);
-		struct object *o = &(commit->object);
+		o = &(commit->object);
 		if (in_merge_bases(b, &commit, 1))
 			add_object_array(o, NULL, &merges);
 	}

^ permalink raw reply related

* Re: [PATCH] allow multiple calls to submodule merge search for the same path
From: Junio C Hamano @ 2011-08-25 23:28 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: Brad King, git
In-Reply-To: <7vwre1gjq1.fsf@alter.siamese.dyndns.org>

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

> Heiko Voigt <hvoigt@hvoigt.net> writes:
>
>> diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
>> index 8f6f2d6..603fb72 100755
>> --- a/t/t7405-submodule-merge.sh
>> +++ b/t/t7405-submodule-merge.sh
>> @@ -269,7 +269,7 @@ test_expect_success 'setup for recursive merge with submodule' '
>>  '
>>  
>>  # merge should leave submodule unmerged in index
>> -test_expect_failure 'recursive merge with submodule' '
>> +test_expect_success 'recursive merge with submodule' '
>>  	(cd merge-recursive &&
>>  	 test_must_fail git merge top-bc &&
>>  	 echo "160000 $(git rev-parse top-cb:sub) 2	sub" > expect2 &&
>
> What is this patch based on?

Ah, nevermind. I figured it out. Thanks.

^ permalink raw reply

* Re: [PATCH] allow multiple calls to submodule merge search for the same path
From: Junio C Hamano @ 2011-08-25 23:22 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: Brad King, git
In-Reply-To: <20110825211144.GA67523@book.hvoigt.net>

Heiko Voigt <hvoigt@hvoigt.net> writes:

> diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
> index 8f6f2d6..603fb72 100755
> --- a/t/t7405-submodule-merge.sh
> +++ b/t/t7405-submodule-merge.sh
> @@ -269,7 +269,7 @@ test_expect_success 'setup for recursive merge with submodule' '
>  '
>  
>  # merge should leave submodule unmerged in index
> -test_expect_failure 'recursive merge with submodule' '
> +test_expect_success 'recursive merge with submodule' '
>  	(cd merge-recursive &&
>  	 test_must_fail git merge top-bc &&
>  	 echo "160000 $(git rev-parse top-cb:sub) 2	sub" > expect2 &&

What is this patch based on?

^ permalink raw reply

* Re: [PATCH] rev-list: Demonstrate breakage with --ancestry-path --all
From: Junio C Hamano @ 2011-08-25 23:08 UTC (permalink / raw)
  To: Brad King; +Cc: git, Heiko Voigt
In-Reply-To: <438ea0b254ccafb3fc9f3431f8f86007cc03132b.1314290439.git.brad.king@kitware.com>

Brad King <brad.king@kitware.com> writes:

> The option added by commit ebdc94f3 (revision: --ancestry-path,
> 2010-04-20) does not work properly in combination with --all, at least
> in the case of a criss-cross merge:
>
>     b---bc
>    / \ /
>   a   X
>    \ / \
>     c---cb

Hmm, what should --ancestry-path do given more than one positive commit to
begin with, let alone --all? I do not think the request itself does not
make sense.

^ permalink raw reply

* Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Junio C Hamano @ 2011-08-25 22:57 UTC (permalink / raw)
  To: Jeff King; +Cc: Boaz Harrosh, git
In-Reply-To: <20110825204047.GA9948@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> If you have any matching attribute line in your own files, it should
> override. So:
>
>   foo/* -diff
>
> will still mark foo/bar.c as binary, even with this change.
>
> Can anyone think of other possible side effects?
>
> Also, any other extensions that would go into such a list? I have no
> idea what the common extension is for something like pascal or csharp.

As long as the builtin ones are the lowest priority fallback, we should be
Ok.

Do we say anywhere that "Ah, this has 'diff' attribute defined, so it must
be text"? If so, we should fix _that_. In other words, having this one
extra entry

	"* diff=default"

in the builtin_attr[] array should be a no-op, I think.

>
>  attr.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index da29c8e..5118a14 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -294,6 +294,18 @@ static void free_attr_elem(struct attr_stack *e)
>  
>  static const char *builtin_attr[] = {
>  	"[attr]binary -diff -text",
> +	"*.html diff=html",
> +	"*.java diff=java",
> +	"*.perl diff=perl",
> +	"*.pl diff=perl",
> +	"*.php diff=php",
> +	"*.py diff=python",
> +	"*.rb diff=ruby",
> +	"*.bib diff=bibtex",
> +	"*.tex diff=tex",
> +	"*.c diff=cpp",
> +	"*.cc diff=cpp",
> +	"*.cxx diff=cpp",
>  	NULL,
>  };

^ permalink raw reply

* [PATCH] .gitattributes: Enable cpp diff parsing for .[ch] files
From: Boaz Harrosh @ 2011-08-25 22:37 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Git Mailing List, linux-kernel


The Linux Kernel source tree is certainly a C language repository.
As a maintainer and code reviewer I would like too, for example:
See function names as hunk headers and not goto labels. And all
the other goodies a language specific diff parser gives me.

Add a .gitattributes file to the Linux tree to enable cpp parsing
of the source files.

People are welcome to add other parsers for other type of files
if needed. (Like Makefile or Kconfig ...)

CC: Jeff King <peff@peff.net>
CC: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 .gitattributes |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
 create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 0000000..6d2b620
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1,2 @@
+*.h diff=cpp
+*.c diff=cpp
-- 
1.7.6

^ permalink raw reply related

* Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Brandon Casey @ 2011-08-25 22:29 UTC (permalink / raw)
  To: Jeff King; +Cc: Boaz Harrosh, git
In-Reply-To: <20110825204047.GA9948@sigill.intra.peff.net>

On 08/25/2011 03:40 PM, Jeff King wrote:
> We already provide sane hunk-header patterns for specific
> languages. However, the user has to manually map common
> extensions to use them. It's not that hard to do, but it's
> an extra step that the user might not even know is an
> option. Let's be nice and do it automatically.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I tried to think of negative side effects.

That's what I worried about when I last touched this code.  Now I'm
thinking "what took us so long to do this!!??".

> Also, any other extensions that would go into such a list?

*.bib diff=bibtex
*.tex diff=tex

*.[Ff] diff=fortran
*.[Ff][0-9][0-9] diff=fortran

GNU fortran currently recognizes .fXX where XX is 90, 95, 03 and 08
and probably enables/disables features based on the respective standard.
[0-9][0-9] would future proof against fortran f13 and f25 as long as
there aren't other extensions that would conflict.

Wikipedia says that .for is an extension for fortran, but I've never
seen that in the wild.  Maybe it's a windows thing (3-char ext).

-Brandon

^ permalink raw reply

* Re: What's cooking in git.git (Aug 2011, #07; Wed, 24)
From: Junio C Hamano @ 2011-08-25 22:27 UTC (permalink / raw)
  To: Heiko Voigt; +Cc: git
In-Reply-To: <20110825215035.GB67523@book.hvoigt.net>

Heiko Voigt <hvoigt@hvoigt.net> writes:

> Hi Junio,
>
> On Wed, Aug 24, 2011 at 05:09:09PM -0700, Junio C Hamano wrote:
>> * fg/submodule-ff-check-before-push (2011-08-20) 2 commits
>>   (merged to 'next' on 2011-08-24 at 398e764)
>>  + push: teach --recurse-submodules the on-demand option
>>  + push: Don't push a repository with unpushed submodules
>>  (this branch uses jc/combine-diff-callback.)
>> 
>> Will aim to merge to "master" by -rc1.
>
> Have you seen my fixes to the tests of this here:
>
> http://article.gmane.org/gmane.comp.version-control.git/179883

Not really. Please send it as a separate patch, as the new _failure
case by itself is worth a comment in the "git log" output.

Thanks.

^ permalink raw reply

* Re: What's cooking in git.git (Aug 2011, #07; Wed, 24)
From: Junio C Hamano @ 2011-08-25 22:22 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20110825202057.GB6165@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Wed, Aug 24, 2011 at 05:09:09PM -0700, Junio C Hamano wrote:
>
>> * jk/add-i-hunk-filter (2011-07-27) 5 commits
>>   (merged to 'next' on 2011-08-11 at 8ff9a56)
>>  + add--interactive: add option to autosplit hunks
>>  + add--interactive: allow negatation of hunk filters
>>  + add--interactive: allow hunk filtering on command line
>>  + add--interactive: factor out regex error handling
>>  + add--interactive: refactor patch mode argument processing
>> 
>> Needs documentation updates.
>
> I think Duy already mentioned this, but you may want to update your
> "what's cooking" note: it needs not just doc updates, but code to
> actually pass the options along from real git commands that use
> add--interactive, like add, checkout, reset, and stash.

Thanks. Also tests are lacking, too. Although I do not necessarily see the
lack of integration with anything but "add" a show-stopper (I consider
"-p" to chekout, reset and stash are "nice to have"), you are correct that
"add -i" and then choosing '[p]atch' gets very confused with

    Patch update>> 
    Use of uninitialized value in split at /git/git-pu/libexec/git-core/git-add--interactive line 742, <STDIN> line 3.
    Unknown option: --
    usage: git [--version] [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
               [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
               [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
               [-c name=value] [--help]
               <command> [<args>]
    Unknown option: --color
    usage: git [--version] [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
               [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
               [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
               [-c name=value] [--help]
               <command> [<args>]

So it certainly seems not ready for the prime time.

>> * jk/generation-numbers (2011-07-14) 7 commits
>>  - limit "contains" traversals based on commit generation
>>  - check commit generation cache validity against grafts
>>  - pretty: support %G to show the generation number of a commit
>>  - commit: add commit_generation function
>>  - add metadata-cache infrastructure
>>  - decorate: allow storing values instead of pointers
>>  - Merge branch 'jk/tag-contains-ab' (early part) into HEAD
>> 
>> The initial "tag --contains" de-pessimization without need for generation
>> numbers is already in; backburnered.
>
> So...what next? I don't really like leaving the contains traversal
> as-is.

Hmm, honestly speaking, I do not see much problem with it. My knee-jerk
reaction is to go with 1.a and if we really want to do something 1.b
perhaps but I suspect "these are bogus" cache wouldn't be so useful by
itself and we may need a bit more information.

>> * jk/http-auth-keyring (2011-08-03) 13 commits
>>   (merged to 'next' on 2011-08-03 at b06e80e)
>>  ...
>> Not urgent; will not be in 1.7.7.
>
> I'm OK with holding this off for another round. I'd really like to get
> more feedback from third-party helper writers. ...

I actually do not think the lack of finer-than-host level granularity a
problem we need to solve before moving forward. IIRC, when accessing
"http://github.com/frotz" and "http://github.com/nitfol", you would key
the authentication material with something like "http://github.com" (or
was it "http:github.com"? the details do not matter for the purpose of
this discussion). If another site that has multiple independent userbases
within the same host, i.e. the user "xyzzy" at "http://gotheb.com/frotz"
and the user "xyzzy" at "http://gotheb.com/nitfol" need to be treated as
separate identities, it obviously is not enoug to use "gotheb.com" as the
look-up key for the authentication material, but in such a case, the user
knows an important piece of information that git can never figure out by
itself, namely, where the authentication domain boundary lies.

We need to add "auth-domain" support, perhaps from the command line option
and configuration, if we ever need to support such a site.

We can consider what you already have as the default case for a more
general "we cut off at the hostname and take that as the auth-domain
boundary unless told otherwise". We may not have the way to "tell
otherwise" yet, but as long as we are reasonably confident that we know
how to extend the system in a backward compatible way, it is not a
show-stopper.

The primary reason why I wanted to hold this topic off was because of the
frequency of bug report we saw this round to topics _after_ they hit the
"master" branch, indicating that not many people are testing "next" during
the development cycle as they used to in olden days.

^ permalink raw reply

* Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Boaz Harrosh @ 2011-08-25 22:01 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric Sunshine, git
In-Reply-To: <20110825210654.GA11077@sigill.intra.peff.net>

On 08/25/2011 02:06 PM, Jeff King wrote:
> On Thu, Aug 25, 2011 at 05:00:51PM -0400, Eric Sunshine wrote:
> 
>>> Also, any other extensions that would go into such a list? I have no
>>> idea what the common extension is for something like pascal or csharp.
>>
>> C# uses extension ".cs".
>>
>> ".cpp" is common, in fact often required, by Windows compilers.
> 
> Thanks, added both to my list.
> 
>> What about ".h" and ".hpp"?
> 
> How well do our cpp patterns do with header files? I imagine they're
> better than the default, but I don't think I've ever really tried
> anything tricky.
> 
> -Peff

Thanks Jeff, thanks everyone! This looks very promising. Specially that
it's all already there and I don't have to code it up.

RTFM time for me now
Boaz

^ permalink raw reply

* Re: git diff annoyance / feature request
From: Boaz Harrosh @ 2011-08-25 21:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git discussion list
In-Reply-To: <7vippljkxs.fsf@alter.siamese.dyndns.org>

On 08/25/2011 01:27 PM, Junio C Hamano wrote:
> Boaz Harrosh <bharrosh@panasas.com> writes:
> 
>> I mean. The label "try_again" is not at all unique in my file. As a
>> reader I would like to see where is that code going to. The function
>> name is a unique file identifier that tells me exactly where the change
>> is going. The label is not. (It's not freaking BASIC)
>>
>> I bet all this was just inherited from diff. Would it be accepted if
>> I send a patch to fix it? What you guys think a goto label makes any
>> sense at all?
> 
> The default tries to mimic what GNU used to do when we added the feature.
> 
> The diff.*.xfuncname configuration variable is there exactly for people
> like you to tweak what we use for hunk headers. Please experiment with it
> and if you come up with a better set of patterns, people may want to copy
> it and use it themselves. we may even consider updating the built-in
> default with your patterns, once they got adopted by wider audiences.
> 

Thanks, I'll investigate it sounds very interesting.

> Personally, I would have to say that the source wouldn't be using too many
> labels with the same name for this behaviour to be problematic, especially
> if it is not freaking BASIC ;-), so...

The Linux Kernel is full of "goto out" or "goto err" its a common error handling
practice. I actually like it because it taps onto a known pattern.

Now the patch tell me @@@ lable out: !! that's not very useful I would say

Thanks I'm sure I can shape it up the way I like it
Boaz

^ permalink raw reply

* Re: [PATCH] check-ref-format --print: Normalize refnames that start with slashes
From: Michael Haggerty @ 2011-08-25 21:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, cmn
In-Reply-To: <7v39gpjk8m.fsf@alter.siamese.dyndns.org>

On 08/25/2011 10:42 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> And add tests that such refnames are accepted and normalized
>> correctly.
> 
> Thanks. I think this is a bit simpler, though [...]

Indeed.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: git-config: case insensitivity for subsections
From: milki @ 2011-08-25 21:57 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20110825205849.GA10384@sigill.intra.peff.net>

On 16:58 Thu 25 Aug     , Jeff King wrote:
> Is there a reason that you can't use the canonical version in your "git
> config" invocation? Or was it simply confusing that it didn't work? I'd
> much prefer to document this limitation in git-config(1) than change the
> code.

This was simply surprising as I was trying to figure out what exactly
case sensitivity meant and how it affacted sections. This definitely
clears this up for me. I'm actually working on a config parser because I
don't think I've seen a complete implementation besides git-config in a
different language.

^ permalink raw reply

* Re: What's cooking in git.git (Aug 2011, #07; Wed, 24)
From: Heiko Voigt @ 2011-08-25 21:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk4a2mjx6.fsf@alter.siamese.dyndns.org>

Hi Junio,

On Wed, Aug 24, 2011 at 05:09:09PM -0700, Junio C Hamano wrote:
> * fg/submodule-ff-check-before-push (2011-08-20) 2 commits
>   (merged to 'next' on 2011-08-24 at 398e764)
>  + push: teach --recurse-submodules the on-demand option
>  + push: Don't push a repository with unpushed submodules
>  (this branch uses jc/combine-diff-callback.)
> 
> Will aim to merge to "master" by -rc1.

Have you seen my fixes to the tests of this here:

http://article.gmane.org/gmane.comp.version-control.git/179883

?

Cheers Heiko

^ permalink raw reply

* Re: [PATCH] update-ref: whitespace fix
From: Junio C Hamano @ 2011-08-25 21:48 UTC (permalink / raw)
  To: pangyanhan; +Cc: git
In-Reply-To: <1314286850-11080-1-git-send-email-pangyanhan@gmail.com>

pangyanhan@gmail.com writes:

> From: Pang Yan Han <pangyanhan@gmail.com>
>
> Signed-off-by: Pang Yan Han <pangyanhan@gmail.com>
> ---
>  builtin/update-ref.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index 76ba1d5..835c62a 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -11,7 +11,7 @@ static const char * const git_update_ref_usage[] = {
>  
>  int cmd_update_ref(int argc, const char **argv, const char *prefix)
>  {
> -	const char *refname, *oldval, *msg=NULL;
> +	const char *refname, *oldval, *msg = NULL;

Heh, thanks. There are some more.

-- >8 --
Subject: [PATCH] whitespace: have SP on both sides of an assignment "="

I've deliberately excluded the borrowed code in compat/nedmalloc
directory.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/remote.c |    2 +-
 commit.c         |    2 +-
 http-push.c      |    2 +-
 read-cache.c     |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 9ff1cac..1fb441c 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1113,7 +1113,7 @@ static int show(int argc, const char **argv)
 			url = states.remote->url;
 			url_nr = states.remote->url_nr;
 		}
-		for (i=0; i < url_nr; i++)
+		for (i = 0; i < url_nr; i++)
 			printf("  Push  URL: %s\n", url[i]);
 		if (!i)
 			printf("  Push  URL: %s\n", "(no URL)");
diff --git a/commit.c b/commit.c
index ac337c7..913dbab 100644
--- a/commit.c
+++ b/commit.c
@@ -515,7 +515,7 @@ void sort_in_topological_order(struct commit_list ** list, int lifo)
 
 		commit = work_item->item;
 		for (parents = commit->parents; parents ; parents = parents->next) {
-			struct commit *parent=parents->item;
+			struct commit *parent = parents->item;
 
 			if (!parent->indegree)
 				continue;
diff --git a/http-push.c b/http-push.c
index 6e8f6d0..376331a 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1655,7 +1655,7 @@ static int delete_remote_branch(const char *pattern, int force)
 		return error("Remote HEAD is not a symref");
 
 	/* Remote branch must not be the remote HEAD */
-	for (i=0; symref && i<MAXDEPTH; i++) {
+	for (i = 0; symref && i < MAXDEPTH; i++) {
 		if (!strcmp(remote_ref->name, symref))
 			return error("Remote branch %s is the current HEAD",
 				     remote_ref->name);
diff --git a/read-cache.c b/read-cache.c
index 4ac9a03..0a64103 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1087,7 +1087,7 @@ static void show_file(const char * fmt, const char * name, int in_porcelain,
 {
 	if (in_porcelain && *first && header_msg) {
 		printf("%s\n", header_msg);
-		*first=0;
+		*first = 0;
 	}
 	printf(fmt, name);
 }
-- 
1.7.6.1.385.gb7fcd0

^ permalink raw reply related

* Re: git-config: case insensitivity for subsections
From: Jeff King @ 2011-08-25 21:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: milki, git
In-Reply-To: <7vpqjti3dq.fsf@alter.siamese.dyndns.org>

On Thu, Aug 25, 2011 at 02:32:17PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I'm not sure it makes sense to do so. I can see how:
> >
> >   [section.SUBSECTION]
> >
> > and
> >
> >   [section.subsection]
> >
> > should be merged. But isn't:
> >
> >   [section "SUBSECTION"]
> >
> > conceptually a different section entirely?
> 
> I still recall getting scolded by Linus after writing [sec.tion]; this was
> way back when he was still active on this list. I essentially was told
> that [sec "tion"] is _the_ only supported way, and [sec.tion] may work but
> it purely does by accident, not by design.

Hmm. It is a little weird that color.branch.local would have to be
spelled:

  [color "branch"]
    local = blue

and that the "branch" must be case-sensitive.

But then, that wouldn't be my first complaint about our config syntax,
which sort of pretends to be hierarchical (with the dot-syntax) but
isn't really. E.g., I'd really much rather it be spelled:

  [color]
    branch.local = blue

> Do we still even list the bogus [section.SUBSECTION] syntax anywhere in
> our docs? If so, we should remove them and if not we simply just should
> deprecate the code to read such input.

It's in Documentation/config.txt. It seems to blame to e136f33
(Documentation/config.txt: Document config file syntax better,
2007-01-22).

-Peff

^ permalink raw reply

* Re: git-config: case insensitivity for subsections
From: Junio C Hamano @ 2011-08-25 21:32 UTC (permalink / raw)
  To: Jeff King; +Cc: milki, git
In-Reply-To: <20110825205849.GA10384@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I'm not sure it makes sense to do so. I can see how:
>
>   [section.SUBSECTION]
>
> and
>
>   [section.subsection]
>
> should be merged. But isn't:
>
>   [section "SUBSECTION"]
>
> conceptually a different section entirely?

I still recall getting scolded by Linus after writing [sec.tion]; this was
way back when he was still active on this list. I essentially was told
that [sec "tion"] is _the_ only supported way, and [sec.tion] may work but
it purely does by accident, not by design.

Do we still even list the bogus [section.SUBSECTION] syntax anywhere in
our docs? If so, we should remove them and if not we simply just should
deprecate the code to read such input.

^ 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