Git development
 help / color / mirror / Atom feed
* [PATCH v5 3/3] log --graph: customize the graph lines with config log.graphColors
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>

If you have a 256 colors terminal (or one with true color support), then
the predefined 12 colors seem limited. On the other hand, you don't want
to draw graph lines with every single color in this mode because the two
colors could look extremely similar. This option allows you to hand pick
the colors you want.

Even with standard terminal, if your background color is neither black
or white, then the graph line may match your background and become
hidden. You can exclude your background color (or simply the colors you
hate) with this.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt |  4 ++++
 graph.c                  | 42 +++++++++++++++++++++++++++++++++++++++---
 t/t4202-log.sh           | 22 ++++++++++++++++++++++
 3 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..33a007b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2003,6 +2003,10 @@ log.follow::
 	i.e. it cannot be used to follow multiple files and does not work well
 	on non-linear history.
 
+log.graphColors::
+	A list of colors, separated by commas, that can be used to draw
+	history lines in `git log --graph`.
+
 log.showRoot::
 	If true, the initial commit will be shown as a big creation event.
 	This is equivalent to a diff against an empty tree.
diff --git a/graph.c b/graph.c
index dd17201..822d40a 100644
--- a/graph.c
+++ b/graph.c
@@ -4,6 +4,7 @@
 #include "graph.h"
 #include "diff.h"
 #include "revision.h"
+#include "argv-array.h"
 
 /* Internal API */
 
@@ -62,6 +63,26 @@ enum graph_state {
 static const char **column_colors;
 static unsigned short column_colors_max;
 
+static void parse_graph_colors_config(struct argv_array *colors, const char *string)
+{
+	const char *end, *start;
+
+	start = string;
+	end = string + strlen(string);
+	while (start < end) {
+		const char *comma = strchrnul(start, ',');
+		char color[COLOR_MAXLEN];
+
+		if (!color_parse_mem(start, comma - start, color))
+			argv_array_push(colors, color);
+		else
+			warning(_("ignore invalid color '%.*s' in log.graphColors"),
+				(int)(comma - start), start);
+		start = comma + 1;
+	}
+	argv_array_push(colors, GIT_COLOR_RESET);
+}
+
 void graph_set_column_colors(const char **colors, unsigned short colors_max)
 {
 	column_colors = colors;
@@ -207,9 +228,24 @@ struct git_graph *graph_init(struct rev_info *opt)
 {
 	struct git_graph *graph = xmalloc(sizeof(struct git_graph));
 
-	if (!column_colors)
-		graph_set_column_colors(column_colors_ansi,
-					column_colors_ansi_max);
+	if (!column_colors) {
+		struct argv_array ansi_colors = {
+			column_colors_ansi,
+			column_colors_ansi_max + 1
+		};
+		struct argv_array *colors = &ansi_colors;
+		char *string;
+
+		if (!git_config_get_string("log.graphcolors", &string)) {
+			static struct argv_array custom_colors = ARGV_ARRAY_INIT;
+			argv_array_clear(&custom_colors);
+			parse_graph_colors_config(&custom_colors, string);
+			free(string);
+			colors = &custom_colors;
+		}
+		/* graph_set_column_colors takes a max-index, not a count */
+		graph_set_column_colors(colors->argv, colors->argc - 1);
+	}
 
 	graph->commit = NULL;
 	graph->revs = opt;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c..0aeabed 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -313,6 +313,28 @@ test_expect_success 'log --graph with merge' '
 	test_cmp expect actual
 '
 
+cat > expect.colors <<\EOF
+*   Merge branch 'side'
+<BLUE>|<RESET><CYAN>\<RESET>
+<BLUE>|<RESET> * side-2
+<BLUE>|<RESET> * side-1
+* <CYAN>|<RESET> Second
+* <CYAN>|<RESET> sixth
+* <CYAN>|<RESET> fifth
+* <CYAN>|<RESET> fourth
+<CYAN>|<RESET><CYAN>/<RESET>
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge with log.graphColors' '
+	test_config log.graphColors ",, blue,invalid-color, cyan, red  , " &&
+	git log --color=always --graph --date-order --pretty=tformat:%s |
+		test_decode_color | sed "s/ *\$//" >actual &&
+	test_cmp expect.colors actual
+'
+
 test_expect_success 'log --raw --graph -m with merge' '
 	git log --raw --graph --oneline -m master | head -n 500 >actual &&
 	grep "initial" actual
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH v5 2/3] color.c: trim leading spaces in color_parse_mem()
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>

Normally color_parse_mem() is called from config parser which trims the
leading spaces already. The new caller in the next patch won't. Let's be
tidy and trim leading spaces too (we already trim trailing spaces before
comma).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 color.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/color.c b/color.c
index a9eadd1..7bb4a96 100644
--- a/color.c
+++ b/color.c
@@ -207,10 +207,15 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 	struct color fg = { COLOR_UNSPECIFIED };
 	struct color bg = { COLOR_UNSPECIFIED };
 
+	while (len > 0 && isspace(*ptr)) {
+		ptr++;
+		len--;
+	}
+
 	if (!len)
 		return -1;
 
-	if (!strncasecmp(value, "reset", len)) {
+	if (!strncasecmp(ptr, "reset", len)) {
 		xsnprintf(dst, end - dst, GIT_COLOR_RESET);
 		return 0;
 	}
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH v5 1/3] color.c: fix color_parse_mem() with value_len == 0
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170119114123.23784-1-pclouds@gmail.com>

In this code we want to match the word "reset". If len is zero,
strncasecmp() will return zero and we incorrectly assume it's "reset" as
a result.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 color.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/color.c b/color.c
index 81c2676..a9eadd1 100644
--- a/color.c
+++ b/color.c
@@ -207,6 +207,9 @@ int color_parse_mem(const char *value, int value_len, char *dst)
 	struct color fg = { COLOR_UNSPECIFIED };
 	struct color bg = { COLOR_UNSPECIFIED };
 
+	if (!len)
+		return -1;
+
 	if (!strncasecmp(value, "reset", len)) {
 		xsnprintf(dst, end - dst, GIT_COLOR_RESET);
 		return 0;
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH v5 0/3] nd/log-graph-configurable-colors
From: Nguyễn Thái Ngọc Duy @ 2017-01-19 11:41 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170109103258.25341-1-pclouds@gmail.com>

v5 moves space trimming to color_parse_mem() from read_graph_colors_config,
which is renamed to parse_graph... because the config reading is moved
back to graph_init.

I think it looks better, but we may be pushing the limits of
argv_array's abuse.

Nguyễn Thái Ngọc Duy (3):
  color.c: fix color_parse_mem() with value_len == 0
  color.c: trim leading spaces in color_parse_mem()
  log --graph: customize the graph lines with config log.graphColors

 Documentation/config.txt |  4 ++++
 color.c                  | 10 +++++++++-
 graph.c                  | 42 +++++++++++++++++++++++++++++++++++++++---
 t/t4202-log.sh           | 22 ++++++++++++++++++++++
 4 files changed, 74 insertions(+), 4 deletions(-)

-- 
2.8.2.524.g6ff3d78


^ permalink raw reply

* Re: [PATCH 0/6] loose-object fsck fixes/tightening
From: John Szakmeister @ 2017-01-19 11:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Dennis Kaarsemaker, git
In-Reply-To: <20170113175258.e66taigy4wpokohk@sigill.intra.peff.net>

On Fri, Jan 13, 2017 at 12:52 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 13, 2017 at 04:15:42AM -0500, John Szakmeister wrote:
>
>> > I did notice another interesting case when looking at this. Fsck ends up
>> > in fsck_loose(), which has the sha1 and path of the loose object. It
>> > passes the sha1 to fsck_sha1(), and ignores the path entirely!
>> >
>> > So if you have a duplicate copy of the object in a pack, we'd actually
>> > find and check the duplicate. This can happen, e.g., if you had a loose
>> > object and fetched a thin-pack which made a copy of the loose object to
>> > complete the pack).
>> >
>> > Probably fsck_loose() should be more picky about making sure we are
>> > reading the data from the loose version we found.
>>
>> Interesting find!  Thanks for the information Peff!
>
> So I figured I would knock this out as a fun morning exercise. But
> sheesh, it turned out to be a slog, because most of the functions rely
> on map_sha1_file() to convert the sha1 to an object path at the lowest
> level.

Yeah, I discovered the same thing when I took a look at it a week or so ago. :-(

> But I finally got something working, so here it is. I found another bug
> on the way, along with a few cleanups. And then I did the trailing
> garbage detection at the end, because by that point I knew right where
> it needed to go. :)

I don't know if my opinion counts for much, but the changes look good to me.

-John

^ permalink raw reply

* Re: fatal: bad revision 'git rm -r --ignore-unmatch -- folder'
From: jean-christophe manciot @ 2017-01-19  8:42 UTC (permalink / raw)
  To: git
In-Reply-To: <CAKcFC3ZN+=o_2t4AawCE0c4Tw+t_XJKTHtEq9d7bv-ht5euPbw@mail.gmail.com>

Also some context information:
Ubuntu 16.10 4.8
git 2.11.0

On Thu, Jan 19, 2017 at 9:32 AM, jean-christophe manciot
<actionmystique@gmail.com> wrote:
> In case you were wondering whether these files were tracked or not:
>
> git-Games# git ls-files Ubuntu/16.04
> Ubuntu/16.04/residualvm_0.3.0~git-1_amd64.deb
> Ubuntu/16.04/scummvm-data_1.8.0_all.deb
> Ubuntu/16.04/scummvm-data_1.9.0_all.deb
> Ubuntu/16.04/scummvm-dbgsym_1.8.2~git20160821.bad86050_amd64.deb
> Ubuntu/16.04/scummvm-dbgsym_1.9.0_amd64.deb
> Ubuntu/16.04/scummvm_1.8.0_amd64.deb
> Ubuntu/16.04/scummvm_1.9.0_amd64.deb
>
> On Tue, Jan 17, 2017 at 4:30 PM, jean-christophe manciot
> <actionmystique@gmail.com> wrote:
>> Hi there,
>>
>> I'm trying to purge a complete folder and its files from the
>> repository history with:
>>
>> git-Games# git filter-branch 'git rm -r --ignore-unmatch --
>> Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
>> fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
>>
>> git does not find the folder although it's there:
>>
>> git-Games# ll Ubuntu/16.04/
>> total 150316
>> drwxr-x--- 2 actionmystique actionmystique     4096 Nov 19 13:40 ./
>> drwxr-x--- 4 actionmystique actionmystique     4096 Oct 30 14:02 ../
>> -rwxr-x--- 1 actionmystique actionmystique  2190394 May  9  2016
>> residualvm_0.3.0~git-1_amd64.deb*
>> ...
>> -rw-r--r-- 1 actionmystique actionmystique 67382492 Oct 13 09:15
>> scummvm-dbgsym_1.9.0_amd64.deb
>>
>> What is going on?
>>
>> --
>> Jean-Christophe
>
>
>
> --
> Jean-Christophe



-- 
Jean-Christophe

^ permalink raw reply

* Re: fatal: bad revision 'git rm -r --ignore-unmatch -- folder'
From: jean-christophe manciot @ 2017-01-19  8:32 UTC (permalink / raw)
  To: git
In-Reply-To: <CAKcFC3aqjLNUNKPDZ08rO_SBkY84uvHBerCxnSchAe8rH0dnMg@mail.gmail.com>

In case you were wondering whether these files were tracked or not:

git-Games# git ls-files Ubuntu/16.04
Ubuntu/16.04/residualvm_0.3.0~git-1_amd64.deb
Ubuntu/16.04/scummvm-data_1.8.0_all.deb
Ubuntu/16.04/scummvm-data_1.9.0_all.deb
Ubuntu/16.04/scummvm-dbgsym_1.8.2~git20160821.bad86050_amd64.deb
Ubuntu/16.04/scummvm-dbgsym_1.9.0_amd64.deb
Ubuntu/16.04/scummvm_1.8.0_amd64.deb
Ubuntu/16.04/scummvm_1.9.0_amd64.deb

On Tue, Jan 17, 2017 at 4:30 PM, jean-christophe manciot
<actionmystique@gmail.com> wrote:
> Hi there,
>
> I'm trying to purge a complete folder and its files from the
> repository history with:
>
> git-Games# git filter-branch 'git rm -r --ignore-unmatch --
> Ubuntu/16.04/' --tag-name-filter cat -- --all HEAD
> fatal: bad revision 'git rm -r --ignore-unmatch -- Ubuntu/16.04/'
>
> git does not find the folder although it's there:
>
> git-Games# ll Ubuntu/16.04/
> total 150316
> drwxr-x--- 2 actionmystique actionmystique     4096 Nov 19 13:40 ./
> drwxr-x--- 4 actionmystique actionmystique     4096 Oct 30 14:02 ../
> -rwxr-x--- 1 actionmystique actionmystique  2190394 May  9  2016
> residualvm_0.3.0~git-1_amd64.deb*
> ...
> -rw-r--r-- 1 actionmystique actionmystique 67382492 Oct 13 09:15
> scummvm-dbgsym_1.9.0_amd64.deb
>
> What is going on?
>
> --
> Jean-Christophe



-- 
Jean-Christophe

^ permalink raw reply

* Re: Git: new feature suggestion
From: Konstantin Khomoutov @ 2017-01-19  6:33 UTC (permalink / raw)
  To: Joao Pinto; +Cc: git, Linus Torvalds, CARLOS.PALMINHA@synopsys.com
In-Reply-To: <4817eb00-6efc-e3c0-53d7-46f2509350d3@synopsys.com>

On Wed, 18 Jan 2017 10:40:52 +0000
Joao Pinto <Joao.Pinto@synopsys.com> wrote:

[...]
> I have seen a lot of Linux developers avoid this re-organization
> operations because they would lose the renamed file history, because
> a new log is created for the new file, even if it is a renamed
> version of itself. I am sending you this e-mail to suggest the
> creation of a new feature in Git: when renamed, a file or folder
> should inherit his parent’s log and a “rename: …” would be
> automatically created or have some kind of pointer to its “old” form
> to make history analysis easier.

Git does not record renames because of its stance that what matters is
code _of the whole project_ as opposed to its location in a particular
file.

Hence with regard to renames Git "works backwards" by detecting them
dynamically while traversing the history (such as with `git log`
etc).  This detection uses certain heuristics which can be controlled
with knobs pointed to by Stefan Beller.

Still, I welcome you to read the sort-of "reference" post by Linus
Torvalds [1] in which he explains the reasoning behind this approach
implemented in Git.  IMO, understanding the reasoning behind the idea
is much better than just mechanically learning how to use it.

The whole thread (esp. Torvalds' replies) is worth reading, but that
particular mail summarizes the whole thing very well.

(The reference link to it used to be [2], but Gmane is not fully
recovered to be able to display it.)

1. http://public-inbox.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/
2. http://thread.gmane.org/gmane.comp.version-control.git/27/focus=217

^ permalink raw reply

* [PATCHv3 3/4] cache.h: document add_[file_]to_index
From: Stefan Beller @ 2017-01-19  3:18 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 cache.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/cache.h b/cache.h
index 929474d7a9..12394eb541 100644
--- a/cache.h
+++ b/cache.h
@@ -614,8 +614,18 @@ extern int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_IGNORE_ERRORS	4
 #define ADD_CACHE_IGNORE_REMOVAL 8
 #define ADD_CACHE_INTENT 16
+/*
+ * These two are used to add the contents of the file at path
+ * to the index, marking the working tree up-to-date by storing
+ * the cached stat info in the resulting cache entry.  A caller
+ * that has already run lstat(2) on the path can call
+ * add_to_index(), and all others can call add_file_to_index();
+ * the latter will do necessary lstat(2) internally before
+ * calling the former.
+ */
 extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
 extern int add_file_to_index(struct index_state *, const char *path, int flags);
+
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
 extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
 extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
-- 
2.11.0.299.g762782ba8a


^ permalink raw reply related

* [PATCHv3 4/4] documentation: retire unfinished documentation
From: Stefan Beller @ 2017-01-19  3:18 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>

When looking for documentation for a specific function, you may be tempted
to run

  git -C Documentation grep index_name_pos

only to find the file technical/api-in-core-index.txt, which doesn't
help for understanding the given function. It would be better to not find
these functions in the documentation, such that people directly dive into
the code instead.

In the previous patches we have documented
* index_name_pos()
* remove_index_entry_at()
* add_[file_]to_index()
in cache.h

We already have documentation for:
* add_index_entry()
* read_index()

Which leaves us with a TODO for:
* cache -> the_index macros
* refresh_index()
* discard_index()
* ie_match_stat() and ie_modified(); how they are different and when to
  use which.
* write_index() that was renamed to write_locked_index
* cache_tree_invalidate_path()
* cache_tree_update()

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/technical/api-in-core-index.txt | 21 ---------------------
 1 file changed, 21 deletions(-)
 delete mode 100644 Documentation/technical/api-in-core-index.txt

diff --git a/Documentation/technical/api-in-core-index.txt b/Documentation/technical/api-in-core-index.txt
deleted file mode 100644
index adbdbf5d75..0000000000
--- a/Documentation/technical/api-in-core-index.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-in-core index API
-=================
-
-Talk about <read-cache.c> and <cache-tree.c>, things like:
-
-* cache -> the_index macros
-* read_index()
-* write_index()
-* ie_match_stat() and ie_modified(); how they are different and when to
-  use which.
-* index_name_pos()
-* remove_index_entry_at()
-* remove_file_from_index()
-* add_file_to_index()
-* add_index_entry()
-* refresh_index()
-* discard_index()
-* cache_tree_invalidate_path()
-* cache_tree_update()
-
-(JC, Linus)
-- 
2.11.0.299.g762782ba8a


^ permalink raw reply related

* [PATCHv3 2/4] cache.h: document remove_index_entry_at
From: Stefan Beller @ 2017-01-19  3:18 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>

Do this by moving the existing documentation from
read-cache.c to cache.h.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 cache.h      | 3 +++
 read-cache.c | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 1469ddeafe..929474d7a9 100644
--- a/cache.h
+++ b/cache.h
@@ -603,7 +603,10 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
 #define ADD_CACHE_KEEP_CACHE_TREE 32	/* Do not invalidate cache-tree */
 extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
 extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
+
+/* Remove entry, return true if there are more entries to go. */
 extern int remove_index_entry_at(struct index_state *, int pos);
+
 extern void remove_marked_cache_entries(struct index_state *istate);
 extern int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_VERBOSE 1
diff --git a/read-cache.c b/read-cache.c
index 2eca639cce..63a414cdb5 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -503,7 +503,6 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel
 	return index_name_stage_pos(istate, name, namelen, 0);
 }
 
-/* Remove entry, return true if there are more entries to go.. */
 int remove_index_entry_at(struct index_state *istate, int pos)
 {
 	struct cache_entry *ce = istate->cache[pos];
-- 
2.11.0.299.g762782ba8a


^ permalink raw reply related

* [PATCHv3 1/4] cache.h: document index_name_pos
From: Stefan Beller @ 2017-01-19  3:18 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170119031854.4570-1-sbeller@google.com>

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 cache.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/cache.h b/cache.h
index 1b67f078dd..1469ddeafe 100644
--- a/cache.h
+++ b/cache.h
@@ -575,7 +575,26 @@ extern int verify_path(const char *path);
 extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
 extern void adjust_dirname_case(struct index_state *istate, char *name);
 extern struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
+
+/*
+ * Searches for an entry defined by name and namelen in the given index.
+ * If the return value is positive (including 0) it is the position of an
+ * exact match. If the return value is negative, the negated value minus 1
+ * is the position where the entry would be inserted.
+ * Example: The current index consists of these files and its stages:
+ *
+ *   b#0, d#0, f#1, f#3
+ *
+ * index_name_pos(&index, "a", 1) -> -1
+ * index_name_pos(&index, "b", 1) ->  0
+ * index_name_pos(&index, "c", 1) -> -2
+ * index_name_pos(&index, "d", 1) ->  1
+ * index_name_pos(&index, "e", 1) -> -3
+ * index_name_pos(&index, "f", 1) -> -3
+ * index_name_pos(&index, "g", 1) -> -5
+ */
 extern int index_name_pos(const struct index_state *, const char *name, int namelen);
+
 #define ADD_CACHE_OK_TO_ADD 1		/* Ok to add */
 #define ADD_CACHE_OK_TO_REPLACE 2	/* Ok to replace file/directory */
 #define ADD_CACHE_SKIP_DFCHECK 4	/* Ok to skip DF conflict checks */
-- 
2.11.0.299.g762782ba8a


^ permalink raw reply related

* [PATCHv3 0/4] start documenting core functions
From: Stefan Beller @ 2017-01-19  3:18 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-2-sbeller@google.com>

v3:
* dropped commets on constants as they were not helpful
* fixed one result in the example for  index_name_pos(&index, "f", 1) (which
  is -3 now)

v2:
included all suggestions from Junio

v1:
The two single patches[1] are turned into a series here.

[1] https://public-inbox.org/git/20170117200147.25425-1-sbeller@google.com/

Thanks,
Stefan

Stefan Beller (4):
  cache.h: document index_name_pos
  cache.h: document remove_index_entry_at
  cache.h: document add_[file_]to_index
  documentation: retire unfinished documentation

 Documentation/technical/api-in-core-index.txt | 21 -------------
 cache.h                                       | 43 +++++++++++++++++++++++----
 read-cache.c                                  |  1 -
 3 files changed, 38 insertions(+), 27 deletions(-)
 delete mode 100644 Documentation/technical/api-in-core-index.txt

-- 
2.11.0.299.g762782ba8a


^ permalink raw reply

* Re: [PATCH] attr: mark a file-local symbol as static
From: Ramsay Jones @ 2017-01-19  1:22 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <20170118230541.GE10641@google.com>



On 18/01/17 23:05, Brandon Williams wrote:
> On 01/18, Ramsay Jones wrote:
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> ---
>>
>> Hi Brandon,
>>
>> If you need to re-roll your 'bw/attr' branch, could you please
>> squash this into the relevant patch (commit 8908457159,
>> "attr: use hashmap for attribute dictionary", 12-01-2017).
>>
>> Also, I note that, although they are declared as part of the
>> public attr api, attr_check_clear() and attr_check_reset() are
>> also not called outside of attr.c. Are these functions part of
>> the public api?
>>
>> Also, a minor point, but attr_check_reset() is called (line 1050)
>> before it's definition (line 1114). This is not a problem, given
>> the declaration in attr.h, but I prefer definitions to come before
>> use, where possible.
>>
>> Thanks!
>>
>> ATB,
>> Ramsay Jones
> 
> Yes of course, I believe Stefan also pointed that out earlier today so I
> have it fixed locally.

Yep, I noticed Stefan's email just a few minutes after I hit
send! ;-)

> For attr_check_clear() and attr_check_reset() the intent is that they
> are the accepted way to either clear or reset the attr_check structure.
> Currently most users of the attribute system don't have a need to clear
> or reset the structure but there could be future callers who need that
> functionality.  If you feel like they shouldn't be part of the api right
> now then I'm open to changing that for this series.

No, I just wanted to check that they were intended to be part of
the public api and that you anticipate additional callers in the
future.

[I would still prefer definitions before use, but many people would
not agree with me, so ...]

ATB,
Ramsay Jones



^ permalink raw reply

* Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
From: Eric Wong @ 2017-01-19  0:37 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Santiago Torres, meta, git, peff, sunshine, walters,
	Lukas Puehringer
In-Reply-To: <20170118201644.GA13758@starla>

Eric Wong <e@80x24.org> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > 
> >   http://public-inbox.org/git/20170118182831.pkhqu2np3bh2puei@LykOS.localdomain/
> 
> Eeep!  This looks like a regression I introduced when working
> around Richard Hansen's S/MIME mails the other week on git@vger:
>
>   https://public-inbox.org/meta/20170110222235.GB27356@dcvr/T/#u

Yep, I copied SUPER and used it improperly in a subclass.  Should be
fixed now, and reimported several messages from my Maildir.  NNTP
readers will see new article numbers for reimported Message-IDs.

^ permalink raw reply

* Re: [PATCHv2 3/4] cache.h: document add_[file_]to_index
From: Stefan Beller @ 2017-01-19  0:08 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <20170119000037.GF10641@google.com>

On Wed, Jan 18, 2017 at 4:00 PM, Brandon Williams <bmwill@google.com> wrote:
> On 01/18, Stefan Beller wrote:
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>  cache.h | 21 ++++++++++++++++-----
>>  1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/cache.h b/cache.h
>> index 87eccdb211..03c46b9b99 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -609,13 +609,24 @@ extern int remove_index_entry_at(struct index_state *, int pos);
>>
>>  extern void remove_marked_cache_entries(struct index_state *istate);
>>  extern int remove_file_from_index(struct index_state *, const char *path);
>> -#define ADD_CACHE_VERBOSE 1
>> -#define ADD_CACHE_PRETEND 2
>> -#define ADD_CACHE_IGNORE_ERRORS      4
>> -#define ADD_CACHE_IGNORE_REMOVAL 8
>> -#define ADD_CACHE_INTENT 16
>> +
>> +#define ADD_CACHE_VERBOSE 1          /* verbose */
>> +#define ADD_CACHE_PRETEND 2          /* dry run */
>> +#define ADD_CACHE_IGNORE_ERRORS 4    /* ignore errors */
>> +#define ADD_CACHE_IGNORE_REMOVAL 8   /* do not remove files from index */
>> +#define ADD_CACHE_INTENT 16          /* intend to add later; stage empty file */
>
> I usually prefer having defines like these use shift operators to set
> the desired bit '(1<<2)' instead of '4', etc.  Is there a preference for
> git as a whole?  I know this is just a documentation change so maybe
> this isn't even the place to discuss this.

eh, and I forgot to remove the comments that Junio thought of as redundant.
I agree that (1<<N)) is usually better than the actual number. But I think
we do not want to change that for the same reason as we don't want to add
these comments there: Digging into history just got more complicated here.
("Who introduced ADD_CACHE_INTENT and why?" you need to skip the
reformatting/adding document patch to actually find the answer.)

Thanks for spotting,
Stefan

>
> --
> Brandon Williams

^ permalink raw reply

* Re: [PATCHv2 3/4] cache.h: document add_[file_]to_index
From: Brandon Williams @ 2017-01-19  0:00 UTC (permalink / raw)
  To: Stefan Beller; +Cc: gitster, git
In-Reply-To: <20170118232145.31606-4-sbeller@google.com>

On 01/18, Stefan Beller wrote:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  cache.h | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/cache.h b/cache.h
> index 87eccdb211..03c46b9b99 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -609,13 +609,24 @@ extern int remove_index_entry_at(struct index_state *, int pos);
>  
>  extern void remove_marked_cache_entries(struct index_state *istate);
>  extern int remove_file_from_index(struct index_state *, const char *path);
> -#define ADD_CACHE_VERBOSE 1
> -#define ADD_CACHE_PRETEND 2
> -#define ADD_CACHE_IGNORE_ERRORS	4
> -#define ADD_CACHE_IGNORE_REMOVAL 8
> -#define ADD_CACHE_INTENT 16
> +
> +#define ADD_CACHE_VERBOSE 1		/* verbose */
> +#define ADD_CACHE_PRETEND 2 		/* dry run */
> +#define ADD_CACHE_IGNORE_ERRORS 4	/* ignore errors */
> +#define ADD_CACHE_IGNORE_REMOVAL 8	/* do not remove files from index */
> +#define ADD_CACHE_INTENT 16		/* intend to add later; stage empty file */

I usually prefer having defines like these use shift operators to set
the desired bit '(1<<2)' instead of '4', etc.  Is there a preference for
git as a whole?  I know this is just a documentation change so maybe
this isn't even the place to discuss this.

-- 
Brandon Williams

^ permalink raw reply

* Re: "git diff --ignore-space-change --stat" lists files with only whitespace differences as "changed"
From: Junio C Hamano @ 2017-01-18 22:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Matt McCutchen, git
In-Reply-To: <20170118210821.xugr6jnvzgoxpynb@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Wed, Jan 18, 2017 at 12:57:12PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > So I dunno. A sensible rule to me is "iff -p would show a diff header,
>> > then --stat should mention it".
>> 
>> True but tricky (you need a better definition of "a diff header").
>> 
>> In addition to a new and deleted file, does a file whose executable
>> bit was flipped need mention?  If so, then "diff --git" is the diff
>> header in the above.  Otherwise "@@ ... @@", iow, "iff -p would show
>> any hunk".
>> 
>> I think the patch implements the latter, which I think is sensible.
>
> I would think the former is more sensible (and is what my patch is
> working towards).

Doh (yes, "diff --git" was what I meant).  As a mode-flipping patch
does not have hunk but does show the header, it wants to be included
in "git diff --stat" output, I would think, independent of -b issue.
In fact

	chmod +x README.md
	git diff --stat

does show a 0-line diffstat.


> Doing:
>
>   >empty
>   git add empty
>   git diff --cached
>
> shows a "diff --git" header, but no hunk. I think it should show a
> diffstat (and does with my patch).
>
> I was thinking the rule should be something like:
>
>   if (p->status == DIFF_STATUS_MODIFIED &&
>       !file->added && !file->deleted))
>
> and otherwise include the entry, since it would be an add, delete,
> rename, etc, which carries useful information.
>
> Though a pure rename would not hit this code path at all, I would think
> (it would not trigger "!same_contents"). And a rename where there was a
> whitespace only change probably _should_ be omitted from "-b".
>
> Ditto for a pure mode change, I think. We do not run the contents
> through diff at all, so it does not hit this code path.
>
> -Peff

^ permalink raw reply

* [PATCHv2 4/4] documentation: retire unfinished documentation
From: Stefan Beller @ 2017-01-18 23:21 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-1-sbeller@google.com>

When looking for documentation for a specific function, you may be tempted
to run

  git -C Documentation grep index_name_pos

only to find the file technical/api-in-core-index.txt, which doesn't
help for understanding the given function. It would be better to not find
these functions in the documentation, such that people directly dive into
the code instead.

In the previous patches we have documented
* index_name_pos()
* remove_index_entry_at()
* add_[file_]to_index()
in cache.h

We already have documentation for:
* add_index_entry()
* read_index()

Which leaves us with a TODO for:
* cache -> the_index macros
* refresh_index()
* discard_index()
* ie_match_stat() and ie_modified(); how they are different and when to
  use which.
* write_index() that was renamed to write_locked_index
* cache_tree_invalidate_path()
* cache_tree_update()

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/technical/api-in-core-index.txt | 21 ---------------------
 1 file changed, 21 deletions(-)
 delete mode 100644 Documentation/technical/api-in-core-index.txt

diff --git a/Documentation/technical/api-in-core-index.txt b/Documentation/technical/api-in-core-index.txt
deleted file mode 100644
index adbdbf5d75..0000000000
--- a/Documentation/technical/api-in-core-index.txt
+++ /dev/null
@@ -1,21 +0,0 @@
-in-core index API
-=================
-
-Talk about <read-cache.c> and <cache-tree.c>, things like:
-
-* cache -> the_index macros
-* read_index()
-* write_index()
-* ie_match_stat() and ie_modified(); how they are different and when to
-  use which.
-* index_name_pos()
-* remove_index_entry_at()
-* remove_file_from_index()
-* add_file_to_index()
-* add_index_entry()
-* refresh_index()
-* discard_index()
-* cache_tree_invalidate_path()
-* cache_tree_update()
-
-(JC, Linus)
-- 
2.11.0.299.g762782ba8a


^ permalink raw reply related

* [PATCHv2 1/4] cache.h: document index_name_pos
From: Stefan Beller @ 2017-01-18 23:21 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-1-sbeller@google.com>

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 cache.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/cache.h b/cache.h
index 1b67f078dd..3dbba69aec 100644
--- a/cache.h
+++ b/cache.h
@@ -575,7 +575,26 @@ extern int verify_path(const char *path);
 extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
 extern void adjust_dirname_case(struct index_state *istate, char *name);
 extern struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
+
+/*
+ * Searches for an entry defined by name and namelen in the given index.
+ * If the return value is positive (including 0) it is the position of an
+ * exact match. If the return value is negative, the negated value minus 1
+ * is the position where the entry would be inserted.
+ * Example: The current index consists of these files and its stages:
+ *
+ *   b#0, d#0, f#1, f#3
+ *
+ * index_name_pos(&index, "a", 1) -> -1
+ * index_name_pos(&index, "b", 1) ->  0
+ * index_name_pos(&index, "c", 1) -> -2
+ * index_name_pos(&index, "d", 1) ->  1
+ * index_name_pos(&index, "e", 1) -> -3
+ * index_name_pos(&index, "f", 1) ->  2
+ * index_name_pos(&index, "g", 1) -> -5
+ */
 extern int index_name_pos(const struct index_state *, const char *name, int namelen);
+
 #define ADD_CACHE_OK_TO_ADD 1		/* Ok to add */
 #define ADD_CACHE_OK_TO_REPLACE 2	/* Ok to replace file/directory */
 #define ADD_CACHE_SKIP_DFCHECK 4	/* Ok to skip DF conflict checks */
-- 
2.11.0.299.g762782ba8a


^ permalink raw reply related

* [PATCHv2 3/4] cache.h: document add_[file_]to_index
From: Stefan Beller @ 2017-01-18 23:21 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-1-sbeller@google.com>

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 cache.h | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 87eccdb211..03c46b9b99 100644
--- a/cache.h
+++ b/cache.h
@@ -609,13 +609,24 @@ extern int remove_index_entry_at(struct index_state *, int pos);
 
 extern void remove_marked_cache_entries(struct index_state *istate);
 extern int remove_file_from_index(struct index_state *, const char *path);
-#define ADD_CACHE_VERBOSE 1
-#define ADD_CACHE_PRETEND 2
-#define ADD_CACHE_IGNORE_ERRORS	4
-#define ADD_CACHE_IGNORE_REMOVAL 8
-#define ADD_CACHE_INTENT 16
+
+#define ADD_CACHE_VERBOSE 1		/* verbose */
+#define ADD_CACHE_PRETEND 2 		/* dry run */
+#define ADD_CACHE_IGNORE_ERRORS 4	/* ignore errors */
+#define ADD_CACHE_IGNORE_REMOVAL 8	/* do not remove files from index */
+#define ADD_CACHE_INTENT 16		/* intend to add later; stage empty file */
+/*
+ * These two are used to add the contents of the file at path
+ * to the index, marking the working tree up-to-date by storing
+ * the cached stat info in the resulting cache entry.  A caller
+ * that has already run lstat(2) on the path can call
+ * add_to_index(), and all others can call add_file_to_index();
+ * the latter will do necessary lstat(2) internally before
+ * calling the former.
+ */
 extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);
 extern int add_file_to_index(struct index_state *, const char *path, int flags);
+
 extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, unsigned int refresh_options);
 extern int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
 extern int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
-- 
2.11.0.299.g762782ba8a


^ permalink raw reply related

* [PATCHv2 0/4] start documenting core functions
From: Stefan Beller @ 2017-01-18 23:21 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170117233503.27137-1-sbeller@google.com>

v2:
included all suggestions from Junio

v1:
The two single patches[1] are turned into a series here.

[1] https://public-inbox.org/git/20170117200147.25425-1-sbeller@google.com/

Thanks,
Stefan

Stefan Beller (4):
  cache.h: document index_name_pos
  cache.h: document remove_index_entry_at
  cache.h: document add_[file_]to_index
  documentation: retire unfinished documentation

 Documentation/technical/api-in-core-index.txt | 21 -------------
 cache.h                                       | 43 +++++++++++++++++++++++----
 read-cache.c                                  |  1 -
 3 files changed, 38 insertions(+), 27 deletions(-)
 delete mode 100644 Documentation/technical/api-in-core-index.txt

-- 
2.11.0.299.g762782ba8a


^ permalink raw reply

* [PATCHv2 2/4] cache.h: document remove_index_entry_at
From: Stefan Beller @ 2017-01-18 23:21 UTC (permalink / raw)
  To: gitster; +Cc: git, Stefan Beller
In-Reply-To: <20170118232145.31606-1-sbeller@google.com>

Do this by moving the existing documentation from
read-cache.c to cache.h.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 cache.h      | 3 +++
 read-cache.c | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 3dbba69aec..87eccdb211 100644
--- a/cache.h
+++ b/cache.h
@@ -603,7 +603,10 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
 #define ADD_CACHE_KEEP_CACHE_TREE 32	/* Do not invalidate cache-tree */
 extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
 extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
+
+/* Remove entry, return true if there are more entries to go. */
 extern int remove_index_entry_at(struct index_state *, int pos);
+
 extern void remove_marked_cache_entries(struct index_state *istate);
 extern int remove_file_from_index(struct index_state *, const char *path);
 #define ADD_CACHE_VERBOSE 1
diff --git a/read-cache.c b/read-cache.c
index 2eca639cce..63a414cdb5 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -503,7 +503,6 @@ int index_name_pos(const struct index_state *istate, const char *name, int namel
 	return index_name_stage_pos(istate, name, namelen, 0);
 }
 
-/* Remove entry, return true if there are more entries to go.. */
 int remove_index_entry_at(struct index_state *istate, int pos)
 {
 	struct cache_entry *ce = istate->cache[pos];
-- 
2.11.0.299.g762782ba8a


^ permalink raw reply related

* [PATCH v4 1/5] doc: add documentation for OPT_STRING_LIST
From: Jacob Keller @ 2017-01-18 23:06 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt, Johannes Schindelin, Junio C Hamano, Jacob Keller
In-Reply-To: <20170118230608.28030-1-jacob.e.keller@intel.com>

From: Jacob Keller <jacob.keller@gmail.com>

Commit c8ba16391655 ("parse-options: add OPT_STRING_LIST helper",
2011-06-09) added the OPT_STRING_LIST as a way to accumulate a repeated
list of strings. However, this was not documented in the
api-parse-options documentation. Add documentation now so that future
developers may learn of its existence.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/technical/api-parse-options.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 27bd701c0d68..36768b479e16 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -168,6 +168,11 @@ There are some macros to easily define options:
 	Introduce an option with string argument.
 	The string argument is put into `str_var`.
 
+`OPT_STRING_LIST(short, long, &struct string_list, arg_str, description)`::
+	Introduce an option with string argument.
+	The string argument is stored as an element in `string_list`.
+	Use of `--no-option` will clear the list of preceding values.
+
 `OPT_INTEGER(short, long, &int_var, description)`::
 	Introduce an option with integer argument.
 	The integer is put into `int_var`.
-- 
2.11.0.488.g1cece4bcb7a5


^ permalink raw reply related

* [PATCH v4 2/5] name-rev: extend --refs to accept multiple patterns
From: Jacob Keller @ 2017-01-18 23:06 UTC (permalink / raw)
  To: git; +Cc: Johannes Sixt, Johannes Schindelin, Junio C Hamano, Jacob Keller
In-Reply-To: <20170118230608.28030-1-jacob.e.keller@intel.com>

From: Jacob Keller <jacob.keller@gmail.com>

Teach git name-rev to take multiple --refs stored as a string list of
patterns. The list of patterns will be matched inclusively, and each ref
only needs to match one pattern to be included. A ref will only be
excluded if it does not match any of the given patterns. Additionally,
if any of the patterns would allow abbreviation, then we will abbreviate
the ref, even if another pattern is more strict and would not have
allowed abbreviation on its own.

Add tests and documentation for this change. The tests expected output
is dynamically generated, but this is in order to avoid hard-coding
a commit object id in the test results (as the expected output is to
simply leave the commit object unnamed).

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 Documentation/git-name-rev.txt       |  4 +++-
 builtin/name-rev.c                   | 42 +++++++++++++++++++++++++-----------
 t/t6007-rev-list-cherry-pick-file.sh | 26 ++++++++++++++++++++++
 3 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index ca28fb8e2a07..7433627db12d 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -26,7 +26,9 @@ OPTIONS
 
 --refs=<pattern>::
 	Only use refs whose names match a given shell pattern.  The pattern
-	can be one of branch name, tag name or fully qualified ref name.
+	can be one of branch name, tag name or fully qualified ref name. If
+	given multiple times, use refs whose names match any of the given shell
+	patterns. Use `--no-refs` to clear any previous ref patterns given.
 
 --all::
 	List all commits reachable from all refs
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index cd89d48b65e8..85897ea1f714 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -108,7 +108,7 @@ static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)
 struct name_ref_data {
 	int tags_only;
 	int name_only;
-	const char *ref_filter;
+	struct string_list ref_filters;
 };
 
 static struct tip_table {
@@ -150,16 +150,34 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
 	if (data->tags_only && !starts_with(path, "refs/tags/"))
 		return 0;
 
-	if (data->ref_filter) {
-		switch (subpath_matches(path, data->ref_filter)) {
-		case -1: /* did not match */
-			return 0;
-		case 0:  /* matched fully */
-			break;
-		default: /* matched subpath */
-			can_abbreviate_output = 1;
-			break;
+	if (data->ref_filters.nr) {
+		struct string_list_item *item;
+		int matched = 0;
+
+		/* See if any of the patterns match. */
+		for_each_string_list_item(item, &data->ref_filters) {
+			/* Check every pattern even after we found a match so
+			 * that we can determine when we should abbreviate the
+			 * output. We will abbreviate the output when any of
+			 * the patterns match a subpath, even if one of the
+			 * patterns matches fully.
+			 */
+			switch (subpath_matches(path, item->string)) {
+			case -1: /* did not match */
+				break;
+			case 0: /* matched fully */
+				matched = 1;
+				break;
+			default: /* matched subpath */
+				matched = 1;
+				can_abbreviate_output = 1;
+				break;
+			}
 		}
+
+		/* If none of the patterns matched, stop now */
+		if (!matched)
+			return 0;
 	}
 
 	add_to_tip_table(oid->hash, path, can_abbreviate_output);
@@ -306,11 +324,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 {
 	struct object_array revs = OBJECT_ARRAY_INIT;
 	int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
-	struct name_ref_data data = { 0, 0, NULL };
+	struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };
 	struct option opts[] = {
 		OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
 		OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
-		OPT_STRING(0, "refs", &data.ref_filter, N_("pattern"),
+		OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
 				   N_("only use refs matching <pattern>")),
 		OPT_GROUP(""),
 		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh
index 1408b608eb03..d9827a6389a3 100755
--- a/t/t6007-rev-list-cherry-pick-file.sh
+++ b/t/t6007-rev-list-cherry-pick-file.sh
@@ -99,6 +99,32 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '
 	test_cmp actual.named expect
 '
 
+test_expect_success 'name-rev multiple --refs combine inclusive' '
+	git rev-list --left-right --cherry-pick F...E -- bar >actual &&
+	git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \
+		<actual >actual.named &&
+	test_cmp actual.named expect
+'
+
+cat >expect <<EOF
+<tags/F
+EOF
+
+test_expect_success 'name-rev --refs excludes non-matched patterns' '
+	git rev-list --left-right --right-only --cherry-pick F...E -- bar >>expect &&
+	git rev-list --left-right --cherry-pick F...E -- bar >actual &&
+	git name-rev --stdin --name-only --refs="*tags/F" \
+		<actual >actual.named &&
+	test_cmp actual.named expect
+'
+
+test_expect_success 'name-rev --no-refs clears the refs list' '
+	git rev-list --left-right --cherry-pick F...E -- bar >expect &&
+	git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
+		<expect >actual &&
+	test_cmp actual expect
+'
+
 cat >expect <<EOF
 +tags/F
 =tags/D
-- 
2.11.0.488.g1cece4bcb7a5


^ permalink raw reply related


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