Git development
 help / color / mirror / Atom feed
* Re: [PATCH v5 0/4] Per-worktree config file support
From: Duy Nguyen @ 2017-01-10 11:41 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Junio C Hamano, Michael J Gruber, Jens Lehmann, Lars Schneider,
	Stefan Beller, Michael Haggerty, Max Kirillov,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>

On Tue, Jan 10, 2017 at 6:25 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Not much has changed from v4, except that the migration to new config
> layout is done automatically _update_ a config variable with "git
> config --worktree".

I accidentally two words that may make it hard to understand this
sentence. It should be "... is done automatically when you update.."
-- 
Duy

^ permalink raw reply

* [PATCH/RFC 5/4] Redefine core.bare in multiple working tree setting
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:33 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>

When core.bare was added, time was simpler, we only had one worktree
associated to one repository. The situation gets a bit complicated when
multiple worktrees are added. If core.bare is set in the per-repo config
file, should all worktrees see this variable?

Since core.bare affects worktree-related commands (e.g. you are not
supposed to run "git status" when core.bare is true because no worktree
is supposed to link to the repository), when multi worktree is added,
core.bare is evaluated true by the main worktree only. Other worktrees
simply do not see core.bare even if it's there.

With per-worktree configuration in place, core.bare is moved to main
worktree's private config file. But it does not really make sense
because this is about _repository_. Instead we could leave core.bare in
the per-repo config and change/extend its definition from:

   If true this repository is assumed to be 'bare' and has no working
   directory associated with it.

to

   If true this repository is assumed to be 'bare' and has no _main_
   working directory associated with it.

In other words, linked worktrees are not covered by core.bare. This
definition is the same as before when it comes to single worktree setup.

A plus of this definition is, it allows a setup where we only have
linked worktrees (e.g. core.bare set to true, and the main repo is
tucked somewhere safe), which makes all worktrees equal again because
"the special one" is gone.

This patch is incomplete. I need to go through all is_bare_repository()
calls and adjust their behavior. But I wanted to run the idea through
the community first..

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt       | 2 +-
 Documentation/git-worktree.txt | 7 +++----
 t/t2029-worktree-config.sh     | 4 ++--
 worktree.c                     | 6 ------
 4 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c508386..ff146be 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -484,7 +484,7 @@ core.preferSymlinkRefs::
 	expect HEAD to be a symbolic link.
 
 core.bare::
-	If true this repository is assumed to be 'bare' and has no
+	If true this repository is assumed to be 'bare' and has no main
 	working directory associated with it.  If this is the case a
 	number of commands that require a working directory will be
 	disabled, such as linkgit:git-add[1] or linkgit:git-merge[1].
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index f5aad0a..a331d0a 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -161,7 +161,7 @@ them to the `config.worktree` of the main working directory. You may
 also take this opportunity to move other configuration that you do not
 want to share to all working directories:
 
- - `core.worktree` and `core.bare` should never be shared
+ - `core.worktree` should never be shared
 
  - `core.sparseCheckout` is recommended per working directory, unless
    you are sure you always use sparse checkout for all working
@@ -169,9 +169,8 @@ want to share to all working directories:
 
 When `git config --worktree` is used to set a configuration variable
 in multiple working directory setup, `extensions.worktreeConfig` will
-be automatically set. The two variables `core.worktree` and
-`core.bare` if present will be moved to `config.worktree` of the main
-working tree.
+be automatically set. The variable `core.worktree` if present will be
+moved to `config.worktree` of the main working tree.
 
 DETAILS
 -------
diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
index 4ebdf13..dc84c94 100755
--- a/t/t2029-worktree-config.sh
+++ b/t/t2029-worktree-config.sh
@@ -70,13 +70,13 @@ test_expect_success 'config.worktree no longer read without extension' '
 	cmp_config -C wt2 shared this.is
 '
 
-test_expect_success 'config --worktree migrate core.bare and core.worktree' '
+test_expect_success 'config --worktree migrate core.worktree' '
 	git config core.bare true &&
 	git config --worktree foo.bar true &&
 	cmp_config true extensions.worktreeConfig &&
 	cmp_config true foo.bar &&
 	cmp_config true core.bare &&
-	! git -C wt1 config core.bare
+	cmp_config -C wt1 true core.bare
 '
 
 test_done
diff --git a/worktree.c b/worktree.c
index d8c9d85..c07cc50 100644
--- a/worktree.c
+++ b/worktree.c
@@ -395,12 +395,6 @@ void migrate_worktree_config(void)
 	read_repository_format(&format, main_path.buf);
 	assert(format.worktree_config == 0);
 
-	if (format.is_bare >= 0) {
-		git_config_set_in_file(worktree_path.buf,
-				       "core.bare", "true");
-		git_config_set_in_file(main_path.buf,
-				       "core.bare", NULL);
-	}
 	if (format.work_tree) {
 		git_config_set_in_file(worktree_path.buf,
 				       "core.worktree",
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH v5 4/4] t2029: add tests for per-worktree config
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, max, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 t/t2029-worktree-config.sh (new +x) | 82 +++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100755 t/t2029-worktree-config.sh

diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
new file mode 100755
index 0000000..4ebdf13
--- /dev/null
+++ b/t/t2029-worktree-config.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description="config file in multi worktree"
+
+. ./test-lib.sh
+
+cmp_config() {
+	if [ "$1" = "-C" ]; then
+		shift &&
+		GD="-C $1" &&
+		shift
+	else
+		GD=
+	fi &&
+	echo "$1" >expected &&
+	shift &&
+	git $GD config "$@" >actual &&
+	test_cmp expected actual
+}
+
+test_expect_success 'setup' '
+	test_commit start &&
+	git config --worktree per.worktree is-ok &&
+	git worktree add wt1 &&
+	git worktree add wt2 &&
+	git config --worktree per.worktree is-ok &&
+	cmp_config true extensions.worktreeConfig
+'
+
+test_expect_success 'config is shared as before' '
+	git config this.is shared &&
+	cmp_config shared this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config is shared (set from another worktree)' '
+	git -C wt1 config that.is also-shared &&
+	cmp_config also-shared that.is &&
+	cmp_config -C wt1 also-shared that.is &&
+	cmp_config -C wt2 also-shared that.is
+'
+
+test_expect_success 'config private to main worktree' '
+	git config --worktree this.is for-main &&
+	cmp_config for-main this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config private to linked worktree' '
+	git -C wt1 config --worktree this.is for-wt1 &&
+	cmp_config for-main this.is &&
+	cmp_config -C wt1 for-wt1 this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'core.bare no longer for main only' '
+	git config core.bare true &&
+	cmp_config true core.bare &&
+	cmp_config -C wt1 true core.bare &&
+	cmp_config -C wt2 true core.bare &&
+	git config --unset core.bare
+'
+
+test_expect_success 'config.worktree no longer read without extension' '
+	git config --unset extensions.worktreeConfig &&
+	cmp_config shared this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config --worktree migrate core.bare and core.worktree' '
+	git config core.bare true &&
+	git config --worktree foo.bar true &&
+	cmp_config true extensions.worktreeConfig &&
+	cmp_config true foo.bar &&
+	cmp_config true core.bare &&
+	! git -C wt1 config core.bare
+'
+
+test_done
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH v5 3/4] config: automatically migrate to new config layout when --worktree is used
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, max, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>

It's not fun to ask the user to set extensions.worktreeConfig manually.
It's error-prone too. So we do it automatically whenever anybody sets a
per-worktree config with "git config" (support for builtin commands is
coming later).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-worktree.txt |  6 ++++++
 builtin/config.c               |  3 ++-
 worktree.c                     | 40 ++++++++++++++++++++++++++++++++++++++++
 worktree.h                     |  6 ++++++
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 329a673..f5aad0a 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -167,6 +167,12 @@ want to share to all working directories:
    you are sure you always use sparse checkout for all working
    directories.
 
+When `git config --worktree` is used to set a configuration variable
+in multiple working directory setup, `extensions.worktreeConfig` will
+be automatically set. The two variables `core.worktree` and
+`core.bare` if present will be moved to `config.worktree` of the main
+working tree.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
diff --git a/builtin/config.c b/builtin/config.c
index 7d390af..9dafefd 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -533,7 +533,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		if (repository_format_worktree_config)
 			given_config_source.file = git_pathdup("config.worktree");
 		else if (worktrees[0] && worktrees[1]) {
-			die("BUG: migration is not supported yet");
+			migrate_worktree_config();
+			given_config_source.file = git_pathdup("config.worktree");
 		} else
 			given_config_source.file = git_pathdup("config");
 		free_worktrees(worktrees);
diff --git a/worktree.c b/worktree.c
index eb61212..d8c9d85 100644
--- a/worktree.c
+++ b/worktree.c
@@ -380,3 +380,43 @@ const struct worktree *find_shared_symref(const char *symref,
 
 	return existing;
 }
+
+void migrate_worktree_config(void)
+{
+	struct strbuf worktree_path = STRBUF_INIT;
+	struct strbuf main_path = STRBUF_INIT;
+	struct repository_format format;
+
+	assert(repository_format_worktree_config == 0);
+
+	strbuf_git_common_path(&worktree_path, "config.worktree");
+	strbuf_git_path(&main_path, "config");
+
+	read_repository_format(&format, main_path.buf);
+	assert(format.worktree_config == 0);
+
+	if (format.is_bare >= 0) {
+		git_config_set_in_file(worktree_path.buf,
+				       "core.bare", "true");
+		git_config_set_in_file(main_path.buf,
+				       "core.bare", NULL);
+	}
+	if (format.work_tree) {
+		git_config_set_in_file(worktree_path.buf,
+				       "core.worktree",
+				       format.work_tree);
+		git_config_set_in_file(main_path.buf,
+				       "core.worktree", NULL);
+	}
+
+	git_config_set_in_file(main_path.buf,
+			       "extensions.worktreeConfig", "true");
+	if (format.version == 0)
+		git_config_set_in_file(main_path.buf,
+				       "core.repositoryFormatVersion", "1");
+
+	repository_format_worktree_config = 1;
+
+	strbuf_release(&main_path);
+	strbuf_release(&worktree_path);
+}
diff --git a/worktree.h b/worktree.h
index d59ce1f..cf82676 100644
--- a/worktree.h
+++ b/worktree.h
@@ -76,4 +76,10 @@ extern const char *worktree_git_path(const struct worktree *wt,
 				     const char *fmt, ...)
 	__attribute__((format (printf, 2, 3)));
 
+/*
+ * Called to add extensions.worktreeConfig to $GIT_DIR/config and move
+ * main worktree specific config variables to $GIT_DIR/config.worktree.
+ */
+extern void migrate_worktree_config(void);
+
 #endif
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH v5 2/4] config: --worktree for manipulating per-worktree config file
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, max, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>

As noted in the previous commit, "git config" without options will read
both per-worktree and per-repo by default. --worktree is needed to read
just per-worktree config. Writing goes to per-repo by default though,
unless --worktree is given.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-config.txt | 22 +++++++++++++++-------
 builtin/config.c             | 15 ++++++++++++++-
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 806873c..ead33a8 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -47,13 +47,15 @@ checks or transformations are performed on the value.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-`--system`, `--global`, `--local` and `--file <filename>` can be
-used to tell the command to read from only that location (see <<FILES>>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file <filename>` can be used to tell the command to read from only
+that location (see <<FILES>>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-`--file <filename>` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file <filename>` can be used to tell the command to
+write to that location (you can say `--local` but that is the
+default).
 
 This command will fail with non-zero status upon error.  Some exit
 codes are:
@@ -133,6 +135,11 @@ from all available files.
 +
 See also <<FILES>>.
 
+--worktree::
+	Similar to `--local` except that `.git/config.worktree` is
+	read from or written to if `extensions.worktreeConfig` is
+	present. If not it's the same as `--local`.
+
 -f config-file::
 --file config-file::
 	Use the given config file instead of the one specified by GIT_CONFIG.
@@ -275,9 +282,10 @@ configuration file. Note that this also affects options like `--replace-all`
 and `--unset`. *'git config' will only ever change one file at a time*.
 
 You can override these rules either by command-line options or by environment
-variables. The `--global` and the `--system` options will limit the file used
-to the global or system-wide file respectively. The `GIT_CONFIG` environment
-variable has a similar effect, but you can specify any filename you want.
+variables. The `--global`, `--system` and `--worktree` options will limit
+the file used to the global, system-wide or per-worktree file respectively.
+The `GIT_CONFIG` environment variable has a similar effect, but you
+can specify any filename you want.
 
 
 ENVIRONMENT
diff --git a/builtin/config.c b/builtin/config.c
index 05843a0..7d390af 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -4,6 +4,7 @@
 #include "parse-options.h"
 #include "urlmatch.h"
 #include "quote.h"
+#include "worktree.h"
 
 static const char *const builtin_config_usage[] = {
 	N_("git config [<options>]"),
@@ -23,6 +24,7 @@ static char key_delim = ' ';
 static char term = '\n';
 
 static int use_global_config, use_system_config, use_local_config;
+static int use_worktree_config;
 static struct git_config_source given_config_source;
 static int actions, types;
 static int end_null;
@@ -56,6 +58,7 @@ static struct option builtin_config_options[] = {
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
 	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
 	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
+	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
 	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
 	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
 	OPT_GROUP(N_("Action")),
@@ -490,6 +493,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (use_global_config + use_system_config + use_local_config +
+	    use_worktree_config +
 	    !!given_config_source.file + !!given_config_source.blob > 1) {
 		error("only one config file at a time.");
 		usage_with_options(builtin_config_usage, builtin_config_options);
@@ -524,7 +528,16 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		given_config_source.file = git_etc_gitconfig();
 	else if (use_local_config)
 		given_config_source.file = git_pathdup("config");
-	else if (given_config_source.file) {
+	else if (use_worktree_config) {
+		struct worktree **worktrees = get_worktrees(0);
+		if (repository_format_worktree_config)
+			given_config_source.file = git_pathdup("config.worktree");
+		else if (worktrees[0] && worktrees[1]) {
+			die("BUG: migration is not supported yet");
+		} else
+			given_config_source.file = git_pathdup("config");
+		free_worktrees(worktrees);
+	} else if (given_config_source.file) {
 		if (!is_absolute_path(given_config_source.file) && prefix)
 			given_config_source.file =
 				xstrdup(prefix_filename(prefix,
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH v5 1/4] config: read per-worktree config files
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, max, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170110112524.12870-1-pclouds@gmail.com>

A new repo extension is added, worktreeConfig. When it is present:

 - Repository config reading by default includes $GIT_DIR/config _and_
   $GIT_DIR/config.worktree. "config" file remains shared in multiple
   worktree setup.

 - The special treatment for core.bare and core.worktree, to stay
   effective only in main worktree, is gone. These config files are
   supposed to be in config.worktree.

This extension is most useful in multiple worktree setup because you
now have an option to store per-worktree config (which is either
.git/config.worktree for main worktree, or
.git/worktrees/xx/config.worktree for linked ones).

This extension can be used in single worktree mode, even though it's
pretty much useless (but this can happen after you remove all linked
worktrees and move back to single worktree).

"git config" reads from both "config" and "config.worktree" by default
(i.e. without either --user, --file...) when this extension is
present. Default writes (not implemented in this patch) still go to
"config", not "config.worktree". A new option --worktree is added for
that (*).

Since a new repo extension is introduced, existing git binaries should
refuse to access to the repo (both from main and linked worktrees). So
they will not misread the config file (i.e. skip the config.worktree
part). They may still accidentally write to the config file anyway if
they use with "git config --file <path>".

This design places a bet on the assumption that the majority of config
variables are shared so it is the default mode. A safer move would be
default writes go to per-worktree file, so that accidental changes are
isolated.

(*) "git config --worktree" points back to "config" file when this
    extension is not present so that it works in any setup.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt               | 11 +++++++++--
 Documentation/git-config.txt           |  4 ++++
 Documentation/git-worktree.txt         | 31 +++++++++++++++++++++++++++++++
 Documentation/gitrepository-layout.txt |  8 ++++++++
 cache.h                                |  2 ++
 config.c                               |  7 +++++++
 environment.c                          |  1 +
 setup.c                                |  5 ++++-
 8 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 30cb946..c508386 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,8 +2,9 @@ CONFIGURATION FILE
 ------------------
 
 The Git configuration file contains a number of variables that affect
-the Git commands' behavior. The `.git/config` file in each repository
-is used to store the configuration for that repository, and
+the Git commands' behavior. The files `.git/config` and optionally
+`config.worktree` (see `extensions.worktreeConfig` below) are each
+repository is used to store the configuration for that repository, and
 `$HOME/.gitconfig` is used to store a per-user configuration as
 fallback values for the `.git/config` file. The file `/etc/gitconfig`
 can be used to store a system-wide default configuration.
@@ -264,6 +265,12 @@ advice.*::
 		show directions on how to proceed from the current state.
 --
 
+extensions.worktreeConfig::
+	If set, by default "git config" reads from both "config" and
+	"config.worktree" file in that order. In multiple working
+	directory mode, "config" file is shared while
+	"config.worktree" is per-working directory.
+
 core.fileMode::
 	Tells Git if the executable bit of files in the working tree
 	is to be honored.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 83f86b9..806873c 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -253,6 +253,10 @@ $XDG_CONFIG_HOME/git/config::
 $GIT_DIR/config::
 	Repository specific configuration file.
 
+$GIT_DIR/config.worktree::
+	This is optional and is only searched when
+	`extensions.worktreeConfig` is present in $GIT_DIR/config.
+
 If no further options are given, all reading options will read all of these
 files that are available. If the global or the system-wide configuration
 file are not available they will be ignored. If the repository configuration
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e257c19..329a673 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -136,6 +136,37 @@ working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+CONFIGURATION FILE
+------------------
+By default, the repository "config" file is shared across all working
+directories. If the config variables `core.bare` or `core.worktree`
+are already present in the config file, they will be applied to the
+main working directory only.
+
+In order to have configuration specific to working directories, you
+can turn on "worktreeConfig" extension, e.g.:
+
+------------
+$ git config extensions.worktreeConfig true
+------------
+
+In this mode, specific configuration stays in the path pointed by `git
+rev-parse --git-path config.worktree`. You can add or update
+configuration in this file with `git config --worktree`. Git before
+version 2.13 will refuse to access repositories with this extension.
+
+Note that in this file, the exception for `core.bare` and
+`core.worktree` is gone. If you have them before, you need to move
+them to the `config.worktree` of the main working directory. You may
+also take this opportunity to move other configuration that you do not
+want to share to all working directories:
+
+ - `core.worktree` and `core.bare` should never be shared
+
+ - `core.sparseCheckout` is recommended per working directory, unless
+   you are sure you always use sparse checkout for all working
+   directories.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index a5f99cb..fee2557 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -143,6 +143,11 @@ config::
 	if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/config" will be
 	used instead.
 
+config.worktree::
+	Working directory specific configuration file for the main
+	working directory in multiple working directory setup (see
+	linkgit:git-worktree[1]).
+
 branches::
 	A slightly deprecated way to store shorthands to be used
 	to specify a URL to 'git fetch', 'git pull' and 'git push'.
@@ -276,6 +281,9 @@ worktrees/<id>/link::
 	file. It is used to detect if the linked repository is
 	manually removed.
 
+worktrees/<id>/config.worktree::
+	Working directory specific configuration file.
+
 SEE ALSO
 --------
 linkgit:git-init[1],
diff --git a/cache.h b/cache.h
index a50a61a..c621a35 100644
--- a/cache.h
+++ b/cache.h
@@ -777,10 +777,12 @@ extern int grafts_replace_parents;
 #define GIT_REPO_VERSION 0
 #define GIT_REPO_VERSION_READ 1
 extern int repository_format_precious_objects;
+extern int repository_format_worktree_config;
 
 struct repository_format {
 	int version;
 	int precious_objects;
+	int worktree_config;
 	int is_bare;
 	char *work_tree;
 	struct string_list unknown_extensions;
diff --git a/config.c b/config.c
index 83fdecb..a461e68 100644
--- a/config.c
+++ b/config.c
@@ -1307,6 +1307,13 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
 	if (repo_config && !access_or_die(repo_config, R_OK, 0))
 		ret += git_config_from_file(fn, repo_config, data);
 
+	if (repository_format_worktree_config) {
+		char *path = git_pathdup("config.worktree");
+		if (!access_or_die(path, R_OK, 0))
+			ret += git_config_from_file(fn, path, data);
+		free(path);
+	}
+
 	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
 	if (git_config_from_parameters(fn, data) < 0)
 		die(_("unable to parse command-line config"));
diff --git a/environment.c b/environment.c
index 0935ec6..df1be15 100644
--- a/environment.c
+++ b/environment.c
@@ -26,6 +26,7 @@ int warn_ambiguous_refs = 1;
 int warn_on_object_refname_ambiguity = 1;
 int ref_paranoia = -1;
 int repository_format_precious_objects;
+int repository_format_worktree_config;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
 const char *apply_default_whitespace;
diff --git a/setup.c b/setup.c
index fe572b8..69efe45 100644
--- a/setup.c
+++ b/setup.c
@@ -389,6 +389,8 @@ static int check_repo_format(const char *var, const char *value, void *vdata)
 			;
 		else if (!strcmp(ext, "preciousobjects"))
 			data->precious_objects = git_config_bool(var, value);
+		else if (!strcmp(ext, "worktreeconfig"))
+			data->worktree_config = git_config_bool(var, value);
 		else
 			string_list_append(&data->unknown_extensions, ext);
 	} else if (strcmp(var, "core.bare") == 0) {
@@ -432,8 +434,9 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 	}
 
 	repository_format_precious_objects = candidate.precious_objects;
+	repository_format_worktree_config = candidate.worktree_config;
 	string_list_clear(&candidate.unknown_extensions, 0);
-	if (!has_common) {
+	if (!has_common || repository_format_worktree_config) {
 		if (candidate.is_bare != -1) {
 			is_bare_repository_cfg = candidate.is_bare;
 			if (is_bare_repository_cfg == 1)
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* [PATCH v5 0/4] Per-worktree config file support
From: Nguyễn Thái Ngọc Duy @ 2017-01-10 11:25 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, git, Jens.Lehmann, larsxschneider, sbeller,
	mhagger, max, Nguyễn Thái Ngọc Duy

Let's get this rolling again. To refresh your memory because it's half
a year since v4 [1], this is about letting each worktree in multi
worktree setup has their own config settings. The most prominent ones
are core.worktree, used by submodules, and core.sparseCheckout.

This time I'm not touching submodules at all. I'm leaving it in the
good hands of "submodule people". All I'm providing is mechanism. How
you use it is up to you. So the series benefits sparse checkout users
only.

Not much has changed from v4, except that the migration to new config
layout is done automatically _update_ a config variable with "git
config --worktree".

I think this one is more or less ready. I have an RFC follow-up patch
about core.bare, but that could be handled separately.

[1] http://public-inbox.org/git/20160720172419.25473-1-pclouds@gmail.com/

Nguyễn Thái Ngọc Duy (4):
  config: read per-worktree config files
  config: --worktree for manipulating per-worktree config file
  config: automatically migrate to new config layout when --worktree is used
  t2029: add tests for per-worktree config

 Documentation/config.txt               | 11 ++++-
 Documentation/git-config.txt           | 26 ++++++++---
 Documentation/git-worktree.txt         | 37 +++++++++++++++
 Documentation/gitrepository-layout.txt |  8 ++++
 builtin/config.c                       | 16 ++++++-
 cache.h                                |  2 +
 config.c                               |  7 +++
 environment.c                          |  1 +
 setup.c                                |  5 ++-
 t/t2029-worktree-config.sh (new +x)    | 82 ++++++++++++++++++++++++++++++++++
 worktree.c                             | 40 +++++++++++++++++
 worktree.h                             |  6 +++
 12 files changed, 230 insertions(+), 11 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

-- 
2.8.2.524.g6ff3d78


^ permalink raw reply

* Re: git branch -D doesn't work with deleted worktree
From: Duy Nguyen @ 2017-01-10 10:47 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Stefan Beller, Roland Illig, git@vger.kernel.org
In-Reply-To: <CA+P7+xqOrRwJsnskfGQpLYyQFjce=4z24zuhrEBd2P4gBLh0Qw@mail.gmail.com>

On Tue, Jan 10, 2017 at 12:08 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Mon, Jan 9, 2017 at 2:07 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Mon, Jan 9, 2017 at 10:44 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>> Why not just update the documentation to be "when you are done with a
>>> work tree you can delete it and then run git worktree prune"?
>>
>> The document does say that (a bit verbose though):
>>
>> When you are done with a linked working tree you can simply delete it.
>> The working tree's administrative files in the repository (see
>> "DETAILS" below) will eventually be removed automatically (see
>> `gc.worktreePruneExpire` in linkgit:git-config[1]), or you can run
>> `git worktree prune` in the main or any linked working tree to
>> clean up any stale administrative files.
>> --
>> Duy
>
> Right, so maybe we can make it more clear since it's not quite as
> simple as "deleting the work tree" because of stuff like stale
> branches..

Patches are welcome. I wrote that paragraph and you could clearly see
its "quality" (from user perspective).

> or would it be worth re-scanning worktrees when we do
> branch deletion? (obviously ignoring the ones that are marked as on
> removable media)

Probably. We run "git worktree prune" as part of "git gc" and that
command is automatically called in a couple places. Adding another
"git gc" here probably does not hurt. However it could trigger a full
repack and make "git branch" hang for a few seconds...

And no I don't want to single out "git worktree prune" to be called in
git-branch. We should either go through git-gc or none. We could check
if a worktree is deleted and suggest the user to run "git gc" though,
but it's getting too complicated when "git worktree remove" will soon
be the recommended way of deleting a worktree. So.. I don't know..
-- 
Duy

^ permalink raw reply

* Re: Test failures when Git is built with libpcre and grep is built without it
From: A. Wilcox @ 2017-01-10 10:36 UTC (permalink / raw)
  To: Jeff King, Andreas Schwab; +Cc: git, musl
In-Reply-To: <20170109213303.4rupe5cqwejfp6af@sigill.intra.peff.net>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

(Hi, musl developers!  Read on for some information regarding what
began as a test failure during packaging of Git 2.7.3 that appears to
be related to musl's regexp library.)


On 09/01/17 05:27, Jeff King wrote:
> Are you trying with a version of git older than v2.7.x?


We are using 2.7.3.  I will admit the version is a bit old, but it is
working on all other arches (and it took an inordinate amount of time
to spin up PowerPC - hooray for toolchain issues).


On 09/01/17 15:33, Jeff King wrote:
> On Mon, Jan 09, 2017 at 02:05:44PM +0100, Andreas Schwab wrote:
>> You need to quote the regexp argument, see the line starting
>> with "test_must_fail" above.
> 
> Oh, duh. I checked that the line in the test was quoted


Guilty of this as well.  Sorry about that.  With the proper
invocation, I receive a success:


elaine trash directory.t7810-grep # git grep -G -F -P -E
"a\x{2b}b\x{2a}c" ab
error: cannot run less: No such file or directory
ab:a+b*c
elaine trash directory.t7810-grep # echo $?
0


Haven't managed to build less(1) for PowerPC yet, but it does seem to
work when quoted.  Yikes!


> The problem is that we are expecting the regex "\x{2b}" to complain
> in regcomp() (as an ERE), but it doesn't. And that probably _is_
> related to musl, which is providing the libc regex (I know this
> looks like a pcre test, but it's checking that "-P -E" overrides
> the pcre option with "-E").
> 
> I'm not sure if musl is wrong for failing to complain about a
> bogus regex. Generally making something that would break into
> something that works is an OK way to extend the standard. So our
> test is at fault for assuming that the regex will fail. I guess
> we'd need to find some more exotic syntax that pcre supports, but
> that ERE doesn't. Maybe "(?:)" or something.
> 
> -Peff
> 


I am cc:ing in the musl development list to see what their thoughts are.

Thanks for the help so far; I really appreciate it.  Hopefully we can
resolve this in a way that makes musl and Git's test suite both more
correct, if necessary.

Happy new year!

- --arw


- -- 
A. Wilcox (awilfox)
Project Lead, Adélie Linux
http://adelielinux.org
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJYdLk+AAoJEMspy1GSK50Uz9kP/2JUSzSSXkKLy3788aAjgBOq
aTPBqKXLCvnTeBpsymXoGueaJzgDnYP6Xi9tUb/j4JAXqaJKGHXTgz8ixsFQJ4SJ
p7NN1JZXsIVGKWQHcxvEXEIRmBK7T9BQ2Hq6qUuk3n4PM6lbD1Ur3G1rUqIM/z76
5cSkvwgvGuOVYXLwSTTprb6EbbZc6WTgcDECFIl4z7eJ3GihChg+EgH4cTDt3xLD
9VHx++hWoEZXued0g0myENjBMeCiFOpYw1bzxn7d2s8masx/If2TkK0CLdexy2ti
hFV/F7g8etgZSdmrmiaJIudTsY6p46/sm/VmGm+8yPIeR8/8EOBlyhKhz4EidNtA
2dpUUJW/RD4uz9yVNjmWhIHaL10weuNLQLd/Tt5O/0dG422vhJxOwwJL9Vhxw70y
1PprD11+ZUJeWsz91C/Bl9CqHqljDP00QX8w/dbHCDsjeKgfUdksy2iKlHBG0sx3
CbL0EcHeE9BLmqmJi9ED7w78cTmNryemK29WFHUGSwzR99Rv9QlMn+4GCF1s0O3N
UKPCueLpPcNvRK+1WHkAwUC0iV97Un3WI6kyExs9khygIHRd1y/yRhbpxibbz337
FJfOTcT7e/YqML8Nb8EdwECRhdGX5u1tKqaOfBwLC62SKGTPi5qXpjaSPPJcJsrZ
phxFt6a45PPECV3ZsEO1
=PMkh
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [ANNOUNCE] git-test: run automated tests against a range of Git commits
From: Jeff King @ 2017-01-10  9:35 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git discussion list
In-Reply-To: <ce6f98a4-1fb7-aa4b-2efb-78d8f49397a7@alum.mit.edu>

On Sun, Jan 08, 2017 at 10:52:25AM +0100, Michael Haggerty wrote:

> > I see the symmetry and simplicity in allowing the user to specify a full
> > range. But it also seems like it's easy to make a mistake that's going
> > to want to test a lot of commits. I wonder if it should complain when
> > there's no lower bound to the commit range. Or alternatively, if there's
> > a single positive reference, treat it as a lower bound, with HEAD as the
> > upper bound (which is vaguely rebase-like).
> 
> I see how this might be unexpected, and it's definitely inconvenient at
> some times (like when you want to test a single commit). I thought it
> would be nice to allow arbitrary `rev-list` expressions (albeit
> currently only a single word), but I think that you are right that other
> semantics would be more convenient.
> 
> I'm thinking of maybe
> 
> * If an argument matches `*..*`, pass it to `rev-list` (like now).
> 
> * Otherwise, treat each argument as a single commit/tree (i.e., pass it
> to `rev-parse`).
> 
> * If no argument is specified, test `@{u}..` (assuming that an
>   upstream is configured). Though actually, this won't be as
>   convenient as it sounds, because (a) `git test` is often run
>   in a separate worktree, and (2) on errors, it currently leaves the
>   repository with a detached `HEAD`.
> 
> * Support a `--stdin` option, to read a list of commits/trees to test
>   from standard input. By this mechanism, users could use arbitrary
>   `rev-list` commands to choose what to test.

That seems quite reasonable to me. The rebase semantics of "a single
argument is my upstream" is convenient, but I think it also confuses
people. Yours seem very simple to explain.

It doesn't allow complex stuff like "git test ^foo ^bar HEAD", but I
don't think "git rev-list ^foo ^bar HEAD | git test --stdin" is really
so bad for complicated cases like that (if anybody would even ever want
such a thing).

> Aside: It would be nice if `git notes` had a subcommand to initialize a
> note reference with an empty tree. (I know how to do it longhand, but
> it's awkward and it should be possible to do it via the `notes` interface.)
> 
> I think ideally `git notes add` would look for pre-existing notes, and:
> 
> * If none are found, create an empty notes reference.
> 
> * If pre-existing notes are found and there was no existing test with
>   that name, probably just leave the old notes in place.
> 
> * If pre-existing notes are found and there was already a test with
>   that name but a different command, perhaps insist that the user
>   decide explicitly whether to forget the old results or continue using
>   them. This might help users avoid the mistake of re-using old results
>   even if they change the manner of testing.

I'm not quite sure what you mean here. By "test" and "command", do you
mean the test name that is used in the notes ref, and the command that
it is defined as?

In the notes-cache.c subsystem, the commit message stores a validity
token which must match in order to use the cache. You could do something
similar here (store the executed command in the commit message, and
invalidate the cache the user has changed the command). The notes-cache
stuff isn't available outside of the C code, though. You could either
expose it, or just do something similar longhand.

Thinking about it, though, I did notice that the tree sha1 is not the
only input to the cache. Things like config.mak (not to mention the
system itself) contribute to the test results. So no system will ever be
perfect, and it seems like just making an easy way to force a retest (or
just invalidate the whole cache) would be sufficient.

But maybe I totally missed what you were trying to say here.

> > It would be even easier if I could just repeat my range and only re-test
> > the "bad" commits. It was then that I decided to actually read the rest
> > of "git test help range" and see that you already wrote such an option,
> > cleverly hidden under the name "--retest".
> 
> I think you were being ironic, but if not, would this have been easier
> to find under another name?

Sorry, the knob on my sarcasm module must have accidentally been knocked
down from "so thick it's obvious even by email".

Yes, I did just mean that I was being blind. You not only had added the
option already, but gave it the exact name I was about to propose it
under. :)

> Yeah, this is awkward, not only because many people don't know what to
> make of detached HEAD, but also because it makes it awkward in general
> to use `git test` in your main working directory. I didn't model this
> behavior on `git rebase --interactive`'s `edit` command, because I
> rarely use that. But I can see how they would fit together pretty well
> for people who like that workflow.

Yeah, after sleeping on it, I think it's best if "git test" remains
separate from that. It's primary function is to run the test, possibly
serving up a cached answer. So it would be perfectly reasonable to do:

  git rebase -x 'git test range HEAD'

to accomplish the interactive testing (though perhaps just "git test"
would be a nice synonym for that).

And then "jump to a thing that I know is broken" becomes a separate
action, whether you are using "git test" or not. I wonder if we could
have:

  git rebase -e HEAD~2

to do an interactive rebase, with "edit" for HEAD~2. I feel like
somebody even proposed that at one point, but I don't think it got
merged.

And then "git test fix" basically becomes:

  git rebase -e "$(git test first-broken)"

though I think you'd still want a shorthand for that.

> > I think it should be possible to script the next steps, though.
> > Something like like "git test fix foo", which would:
> > 
> >   - expand the range of foo@{u}..foo to get the list of commits
> > 
> >   - see which ones were marked as broken
> > 
> >   - kick off an interactive rebase, but override GIT_EDITOR to mark any
> >     broken ones as "edit" instead of "pick"
> > 
> > That lets you separate the act of testing from the act of fixing. You
> > can let the tester run continuously in the background, and only stop to
> > fix when you're at an appropriate point in your work.
> 
> I think you would usually only want to mark only the *first* broken
> commit as "edit", because often errors cascade to descendant commits.

Yeah, I had a similar thought. OTOH, if you didn't say "--keep-going",
you'd only have one breakage either way. I doubt it matters that much in
practice, and either behavior would probably be fine until somebody
comes up with a good use for the multi-commit case.

-Peff

^ permalink raw reply

* Re: Preserve/Prune Old Pack Files
From: Jeff King @ 2017-01-10  9:14 UTC (permalink / raw)
  To: Martin Fick; +Cc: repo-discuss, jmelvin, jgit-dev, git
In-Reply-To: <1802755.dVE0WDQbpH@mfick1-lnx>

On Mon, Jan 09, 2017 at 09:17:56AM -0700, Martin Fick wrote:

> > I suspect the name-change will break a few tools that you
> > might want to use to look at a preserved pack (like
> > verify-pack).  I know that's not your primary use case,
> > but it seems plausible that somebody may one day want to
> > use a preserved pack to try to recover from corruption. I
> > think "git index-pack --stdin
> > <objects/packs/preserved/pack-123.old-pack" could always
> > be a last-resort for re-admitting the objects to the
> > repository.
> 
> or even a simple manual rename/move back to its orginal 
> place?

Yes, that would work. There's not a tool to do it, but it's a fairly
straightforward transformation.

> [loose objects]
> Where would you suggest we store those?  Maybe under 
> ".git/objects/preserved/<xx>/<sha1>"?  Do they need to be 
> renamed also somehow to avoid a find?

It would make sense to me to have a single "preserved" root, with
"<xx>/<sha1>.old" and "packs/pack-<sha1>.old-pack" together under it.

You could also move the objects out of objects/ entirely. Say, to
".git/preserved-objects" or something. Then you could probably do away
with the filename munging altogether, and "restoring" an object or pack
would be a simple "mv" or "cp" (or you could even add preserved-objects
to $GIT_ALTERNATE_OBJECT_DIRECTORIES if you wanted to do a single
operation looking at both sets).

That's all outside the scope of your original purpose (which I think was
just to keep the files _somewhere_ so that the open descriptor stays
valid on NFS). But maybe it would make other related things more
convenient. I dunno. I'm just speaking off the top of my head.

> > That's _way_ more complicated than your problem, and as I
> > said, I do not have a finished solution. But it seems
> > like they touch on a similar concept (a post-delete
> > holding area for objects). So I thought I'd mention it in
> > case if spurs any brilliance.
> 
> I agree, this is a problem I have wanted to solve also.  I 
> think having a "preserved" directory does open the door to 
> such "recovery" solutions, although I think you would 
> actually want to modify the many read code paths to fall 
> back to looking at the preserved area and performing 
> immediate "recovery" of the pack file if it ends up being 
> needed.

In my (admittedly not very concrete) plan, the read code paths
_wouldn't_ know to look in the preserved area. It would be up to the
repacking process to rollback in case of a race. That does open a period
(between the faux delete and the rollback) where readers may be broken.
But that's much better than the state today, which is that the readers
are broken, and that breakage persists forever.

But there may be other better ways of doing it.  What we're really
talking about is a transactional system where neither side locks (or at
least not for an appreciable amount of time), and one side is capable of
falling back and modifying its operation when there's a relevant race.
There's probably some research in this area and some standard solutions,
but it's not an area I'm overly familiar with (and building any solution
on top of POSIX filesystem semantics adds an extra challenge).

> That's a lot of work, but having the packs (and 
> eventually the loose objects) preserved into a location 
> where no new references will be built to depend on them is 
> likely the first step.  Does the name "preserved" do well for 
> that use case also, or would there be some better name, what 
> would a transactional system call them?

I wasn't going to bikeshed, but since you ask...:)

"preserved" to me sounds like something we'd be keeping forever. These
objects are more in a "pending delete" state, or a purgatory. Maybe
something along those lines would be more appropriate.

-Peff

^ permalink raw reply

* Re: [RFC PATCH 0/5] Localise error headers
From: Jeff King @ 2017-01-10  9:04 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <8d0966d0-1ef1-3d1e-95f5-6e6c1ad50536@drmicha.warpmail.net>

On Mon, Jan 09, 2017 at 01:43:15PM +0100, Michael J Gruber wrote:

> > I can't say I'm excited about having matching "_" variants for each
> > function. Are we sure that they are necessary? I.e., would it be
> > acceptable to just translate them always?
> 
> We would still need to mark the strings, e.g.
> 
> die(N_("oopsie"));
> 
> and would not be able to opt out of translating in the code (only in the
> po file, by not providing a translation).

I meant more along the lines of: would it be OK to just always translate
the prefix, even if the message itself is not translated? I.e.,

diff --git a/usage.c b/usage.c
index 82ff13163..8e5400f57 100644
--- a/usage.c
+++ b/usage.c
@@ -32,7 +32,7 @@ static NORETURN void usage_builtin(const char *err, va_list params)
 
 static NORETURN void die_builtin(const char *err, va_list params)
 {
-	vreportf("fatal: ", err, params);
+	vreportf(_("fatal: "), err, params);
 	exit(128);
 }

> In any case, the question is whether we want to tell the user
> 
> A: B
> 
> where A is in English and B is localised, or rather localise both A and
> B (for A in "error", "fatal", "warning"...).
> 
> For localising A and B, we'd need this series or something similar. For
> keeping the mix, we don't need to do anything ;)

What I wrote above would keep the mix, but switch it in the other
direction.

And then presumably that mix would gradually move to 100% consistency as
more messages are translated. But the implicit question is: are there
die() messages that should never be translated? I'm not sure.

-Peff

^ permalink raw reply related

* [PATCH v10 13/20] ref-filter: rename the 'strip' option to 'lstrip'
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

In preparation for the upcoming patch, where we introduce the 'rstrip'
option. Rename the 'strip' option to 'lstrip' to remove ambiguity.

Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.txt | 10 +++++-----
 builtin/tag.c                      |  4 ++--
 ref-filter.c                       | 20 ++++++++++----------
 t/t6300-for-each-ref.sh            | 22 +++++++++++-----------
 4 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index b18eabd69..b0d94deea 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -95,9 +95,9 @@ refname::
 	The name of the ref (the part after $GIT_DIR/).
 	For a non-ambiguous short name of the ref append `:short`.
 	The option core.warnAmbiguousRefs is used to select the strict
-	abbreviation mode. If `strip=<N>` is appended, strips `<N>`
+	abbreviation mode. If `lstrip=<N>` is appended, strips `<N>`
 	slash-separated path components from the front of the refname
-	(e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
+	(e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo`.
 	`<N>` must be a positive integer.  If a displayed ref has fewer
 	components than `<N>`, the command aborts with an error.
 
@@ -116,7 +116,7 @@ objectname::
 
 upstream::
 	The name of a local ref which can be considered ``upstream''
-	from the displayed ref. Respects `:short` and `:strip` in the
+	from the displayed ref. Respects `:short` and `:lstrip` in the
 	same way as `refname` above.  Additionally respects `:track`
 	to show "[ahead N, behind M]" and `:trackshort` to show the
 	terse version: ">" (ahead), "<" (behind), "<>" (ahead and
@@ -130,7 +130,7 @@ upstream::
 
 push::
 	The name of a local ref which represents the `@{push}`
-	location for the displayed ref. Respects `:short`, `:strip`,
+	location for the displayed ref. Respects `:short`, `:lstrip`,
 	`:track`, and `:trackshort` options as `upstream`
 	does. Produces an empty string if no `@{push}` ref is
 	configured.
@@ -174,7 +174,7 @@ if::
 symref::
 	The ref which the given symbolic ref refers to. If not a
 	symbolic ref, nothing is printed. Respects the `:short` and
-	`:strip` options in the same way as `refname` above.
+	`:lstrip` options in the same way as `refname` above.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/builtin/tag.c b/builtin/tag.c
index 73df72811..b4789cec4 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -45,11 +45,11 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
 	if (!format) {
 		if (filter->lines) {
 			to_free = xstrfmt("%s %%(contents:lines=%d)",
-					  "%(align:15)%(refname:strip=2)%(end)",
+					  "%(align:15)%(refname:lstrip=2)%(end)",
 					  filter->lines);
 			format = to_free;
 		} else
-			format = "%(refname:strip=2)";
+			format = "%(refname:lstrip=2)";
 	}
 
 	verify_ref_format(format);
diff --git a/ref-filter.c b/ref-filter.c
index 9140539df..e0015c60f 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -33,8 +33,8 @@ struct if_then_else {
 };
 
 struct refname_atom {
-	enum { R_NORMAL, R_SHORT, R_STRIP } option;
-	unsigned int strip;
+	enum { R_NORMAL, R_SHORT, R_LSTRIP } option;
+	unsigned int lstrip;
 };
 
 /*
@@ -91,10 +91,10 @@ static void refname_atom_parser_internal(struct refname_atom *atom,
 		atom->option = R_NORMAL;
 	else if (!strcmp(arg, "short"))
 		atom->option = R_SHORT;
-	else if (skip_prefix(arg, "strip=", &arg)) {
-		atom->option = R_STRIP;
-		if (strtoul_ui(arg, 10, &atom->strip) || atom->strip <= 0)
-			die(_("positive value expected refname:strip=%s"), arg);
+	else if (skip_prefix(arg, "lstrip=", &arg)) {
+		atom->option = R_LSTRIP;
+		if (strtoul_ui(arg, 10, &atom->lstrip) || atom->lstrip <= 0)
+			die(_("positive value expected refname:lstrip=%s"), arg);
 	} else
 		die(_("unrecognized %%(%s) argument: %s"), name, arg);
 }
@@ -1091,7 +1091,7 @@ static inline char *copy_advance(char *dst, const char *src)
 	return dst;
 }
 
-static const char *strip_ref_components(const char *refname, unsigned int len)
+static const char *lstrip_ref_components(const char *refname, unsigned int len)
 {
 	long remaining = len;
 	const char *start = refname;
@@ -1099,7 +1099,7 @@ static const char *strip_ref_components(const char *refname, unsigned int len)
 	while (remaining) {
 		switch (*start++) {
 		case '\0':
-			die(_("ref '%s' does not have %ud components to :strip"),
+			die(_("ref '%s' does not have %ud components to :lstrip"),
 			    refname, len);
 		case '/':
 			remaining--;
@@ -1113,8 +1113,8 @@ static const char *show_ref(struct refname_atom *atom, const char *refname)
 {
 	if (atom->option == R_SHORT)
 		return shorten_unambiguous_ref(refname, warn_ambiguous_refs);
-	else if (atom->option == R_STRIP)
-		return strip_ref_components(refname, atom->strip);
+	else if (atom->option == R_LSTRIP)
+		return lstrip_ref_components(refname, atom->lstrip);
 	else
 		return refname;
 }
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c53808424..5eb013ca2 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -51,14 +51,14 @@ test_atom() {
 
 test_atom head refname refs/heads/master
 test_atom head refname:short master
-test_atom head refname:strip=1 heads/master
-test_atom head refname:strip=2 master
+test_atom head refname:lstrip=1 heads/master
+test_atom head refname:lstrip=2 master
 test_atom head upstream refs/remotes/origin/master
 test_atom head upstream:short origin/master
-test_atom head upstream:strip=2 origin/master
+test_atom head upstream:lstrip=2 origin/master
 test_atom head push refs/remotes/myfork/master
 test_atom head push:short myfork/master
-test_atom head push:strip=1 remotes/myfork/master
+test_atom head push:lstrip=1 remotes/myfork/master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname $(git rev-parse refs/heads/master)
@@ -141,14 +141,14 @@ test_expect_success 'Check invalid atoms names are errors' '
 	test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
 '
 
-test_expect_success 'arguments to :strip must be positive integers' '
-	test_must_fail git for-each-ref --format="%(refname:strip=0)" &&
-	test_must_fail git for-each-ref --format="%(refname:strip=-1)" &&
-	test_must_fail git for-each-ref --format="%(refname:strip=foo)"
+test_expect_success 'arguments to :lstrip must be positive integers' '
+	test_must_fail git for-each-ref --format="%(refname:lstrip=0)" &&
+	test_must_fail git for-each-ref --format="%(refname:lstrip=-1)" &&
+	test_must_fail git for-each-ref --format="%(refname:lstrip=foo)"
 '
 
 test_expect_success 'stripping refnames too far gives an error' '
-	test_must_fail git for-each-ref --format="%(refname:strip=3)"
+	test_must_fail git for-each-ref --format="%(refname:lstrip=3)"
 '
 
 test_expect_success 'Check format specifiers are ignored in naming date atoms' '
@@ -630,8 +630,8 @@ cat >expected <<EOF
 master
 EOF
 
-test_expect_success 'Verify usage of %(symref:strip) atom' '
-	git for-each-ref --format="%(symref:strip=2)" refs/heads/sym > actual &&
+test_expect_success 'Verify usage of %(symref:lstrip) atom' '
+	git for-each-ref --format="%(symref:lstrip=2)" refs/heads/sym > actual &&
 	test_cmp expected actual
 '
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 12/20] ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

Use the recently introduced refname_atom_parser_internal() within
remote_ref_atom_parser(), this provides a common base for all the ref
printing atoms, allowing %(upstream) and %(push) to also use the
':strip' option.

The atoms '%(push)' and '%(upstream)' will retain the ':track' and
':trackshort' atom modifiers to themselves as they have no meaning in
context to the '%(refname)' and '%(symref)' atoms.

Update the documentation and tests to reflect the same.

Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.txt | 31 ++++++++++++++++---------------
 ref-filter.c                       | 26 +++++++++++++++-----------
 t/t6300-for-each-ref.sh            |  2 ++
 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 5de22cf64..b18eabd69 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -116,23 +116,24 @@ objectname::
 
 upstream::
 	The name of a local ref which can be considered ``upstream''
-	from the displayed ref. Respects `:short` in the same way as
-	`refname` above.  Additionally respects `:track` to show
-	"[ahead N, behind M]" and `:trackshort` to show the terse
-	version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-	or "=" (in sync). `:track` also prints "[gone]" whenever
-	unknown upstream ref is encountered. Append `:track,nobracket`
-	to show tracking information without brackets (i.e "ahead N,
-	behind M").  Has no effect if the ref does not have tracking
-	information associated with it.  All the options apart from
-	`nobracket` are mutually exclusive, but if used together the
-	last option is selected.
+	from the displayed ref. Respects `:short` and `:strip` in the
+	same way as `refname` above.  Additionally respects `:track`
+	to show "[ahead N, behind M]" and `:trackshort` to show the
+	terse version: ">" (ahead), "<" (behind), "<>" (ahead and
+	behind), or "=" (in sync). `:track` also prints "[gone]"
+	whenever unknown upstream ref is encountered. Append
+	`:track,nobracket` to show tracking information without
+	brackets (i.e "ahead N, behind M").  Has no effect if the ref
+	does not have tracking information associated with it.  All
+	the options apart from `nobracket` are mutually exclusive, but
+	if used together the last option is selected.
 
 push::
-	The name of a local ref which represents the `@{push}` location
-	for the displayed ref. Respects `:short`, `:track`, and
-	`:trackshort` options as `upstream` does. Produces an empty
-	string if no `@{push}` ref is configured.
+	The name of a local ref which represents the `@{push}`
+	location for the displayed ref. Respects `:short`, `:strip`,
+	`:track`, and `:trackshort` options as `upstream`
+	does. Produces an empty string if no `@{push}` ref is
+	configured.
 
 HEAD::
 	'*' if HEAD matches current ref (the checked out branch), ' '
diff --git a/ref-filter.c b/ref-filter.c
index 7bfd65633..9140539df 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -54,7 +54,8 @@ static struct used_atom {
 		char color[COLOR_MAXLEN];
 		struct align align;
 		struct {
-			enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT } option;
+			enum { RR_REF, RR_TRACK, RR_TRACKSHORT } option;
+			struct refname_atom refname;
 			unsigned int nobracket : 1;
 		} remote_ref;
 		struct {
@@ -104,7 +105,9 @@ static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 	int i;
 
 	if (!arg) {
-		atom->u.remote_ref.option = RR_NORMAL;
+		atom->u.remote_ref.option = RR_REF;
+		refname_atom_parser_internal(&atom->u.remote_ref.refname,
+					     arg, atom->name);
 		return;
 	}
 
@@ -114,16 +117,17 @@ static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 	for (i = 0; i < params.nr; i++) {
 		const char *s = params.items[i].string;
 
-		if (!strcmp(s, "short"))
-			atom->u.remote_ref.option = RR_SHORTEN;
-		else if (!strcmp(s, "track"))
+		if (!strcmp(s, "track"))
 			atom->u.remote_ref.option = RR_TRACK;
 		else if (!strcmp(s, "trackshort"))
 			atom->u.remote_ref.option = RR_TRACKSHORT;
 		else if (!strcmp(s, "nobracket"))
 			atom->u.remote_ref.nobracket = 1;
-		else
-			die(_("unrecognized format: %%(%s)"), atom->name);
+		else {
+			atom->u.remote_ref.option = RR_REF;
+			refname_atom_parser_internal(&atom->u.remote_ref.refname,
+						     arg, atom->name);
+		}
 	}
 
 	string_list_clear(&params, 0);
@@ -1119,8 +1123,8 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 				    struct branch *branch, const char **s)
 {
 	int num_ours, num_theirs;
-	if (atom->u.remote_ref.option == RR_SHORTEN)
-		*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
+	if (atom->u.remote_ref.option == RR_REF)
+		*s = show_ref(&atom->u.remote_ref.refname, refname);
 	else if (atom->u.remote_ref.option == RR_TRACK) {
 		if (stat_tracking_info(branch, &num_ours,
 				       &num_theirs, NULL)) {
@@ -1152,8 +1156,8 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 			*s = ">";
 		else
 			*s = "<>";
-	} else /* RR_NORMAL */
-		*s = refname;
+	} else
+		die("BUG: unhandled RR_* enum");
 }
 
 char *get_head_description(void)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 18a9e2565..c53808424 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -55,8 +55,10 @@ test_atom head refname:strip=1 heads/master
 test_atom head refname:strip=2 master
 test_atom head upstream refs/remotes/origin/master
 test_atom head upstream:short origin/master
+test_atom head upstream:strip=2 origin/master
 test_atom head push refs/remotes/myfork/master
 test_atom head push:short myfork/master
+test_atom head push:strip=1 remotes/myfork/master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname $(git rev-parse refs/heads/master)
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 14/20] ref-filter: Do not abruptly die when using the 'lstrip=<N>' option
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

Currently when we use the 'lstrip=<N>' option, if 'N' is greater than
the number of components available in the refname, we abruptly end
program execution by calling die().

This behavior is undesired since a single refname with few components
could end program execution. To avoid this, return an empty string
whenever the value 'N' is greater than the number of components
available, instead of calling die().

Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.txt | 3 +--
 ref-filter.c                       | 3 +--
 t/t6300-for-each-ref.sh            | 4 ----
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index b0d94deea..04ffc3552 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -98,8 +98,7 @@ refname::
 	abbreviation mode. If `lstrip=<N>` is appended, strips `<N>`
 	slash-separated path components from the front of the refname
 	(e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo`.
-	`<N>` must be a positive integer.  If a displayed ref has fewer
-	components than `<N>`, the command aborts with an error.
+	`<N>` must be a positive integer.
 
 objecttype::
 	The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/ref-filter.c b/ref-filter.c
index e0015c60f..76553ebc4 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1099,8 +1099,7 @@ static const char *lstrip_ref_components(const char *refname, unsigned int len)
 	while (remaining) {
 		switch (*start++) {
 		case '\0':
-			die(_("ref '%s' does not have %ud components to :lstrip"),
-			    refname, len);
+			return "";
 		case '/':
 			remaining--;
 			break;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 5eb013ca2..d3d1a97db 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -147,10 +147,6 @@ test_expect_success 'arguments to :lstrip must be positive integers' '
 	test_must_fail git for-each-ref --format="%(refname:lstrip=foo)"
 '
 
-test_expect_success 'stripping refnames too far gives an error' '
-	test_must_fail git for-each-ref --format="%(refname:lstrip=3)"
-'
-
 test_expect_success 'Check format specifiers are ignored in naming date atoms' '
 	git for-each-ref --format="%(authordate)" refs/heads &&
 	git for-each-ref --format="%(authordate:default) %(authordate)" refs/heads &&
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 18/20] branch, tag: use porcelain output
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

Call ref-filter's setup_ref_filter_porcelain_msg() to enable
translated messages for the %(upstream:tack) atom. Although branch.c
doesn't currently use ref-filter's printing API's, this will ensure
that when it does in the future patches, we do not need to worry about
translation.

Written-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/branch.c | 2 ++
 builtin/tag.c    | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6423ebce5..34cd61cd9 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -649,6 +649,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	setup_ref_filter_porcelain_msg();
+
 	memset(&filter, 0, sizeof(filter));
 	filter.kind = FILTER_REFS_BRANCHES;
 	filter.abbrev = -1;
diff --git a/builtin/tag.c b/builtin/tag.c
index b4789cec4..8a1a476db 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -375,6 +375,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
+	setup_ref_filter_porcelain_msg();
+
 	git_config(git_tag_config, sorting_tail);
 
 	memset(&opt, 0, sizeof(opt));
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 19/20] branch: use ref-filter printing APIs
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

Port branch.c to use ref-filter APIs for printing. This clears out
most of the code used in branch.c for printing and replaces them with
calls made to the ref-filter library.

Introduce build_format() which gets the format required for printing
of refs. Make amendments to print_ref_list() to reflect these changes.

The strings included in build_format() may not be safely quoted for
inclusion (i.e. it might contain '%' which needs to be escaped with an
additional '%'). Introduce quote_literal_for_format() as a helper
function which takes a string and returns a version of the string that
is safely quoted to be used in the for-each-ref format which is built
in build_format().

Change calc_maxwidth() to also account for the length of HEAD ref, by
calling ref-filter:get_head_discription().

Also change the test in t6040 to reflect the changes.

Before this patch, all cross-prefix symrefs weren't shortened. Since
we're using ref-filter APIs, we shorten all symrefs by default. We also
allow the user to change the format if needed with the introduction of
the '--format' option in the next patch.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/branch.c         | 247 ++++++++++++++++-------------------------------
 t/t3203-branch-output.sh |   2 +-
 t/t6040-tracking-info.sh |   2 +-
 3 files changed, 87 insertions(+), 164 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 34cd61cd9..f293ee5b0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -37,11 +37,11 @@ static unsigned char head_sha1[20];
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,
-	GIT_COLOR_NORMAL,	/* PLAIN */
-	GIT_COLOR_RED,		/* REMOTE */
-	GIT_COLOR_NORMAL,	/* LOCAL */
-	GIT_COLOR_GREEN,	/* CURRENT */
-	GIT_COLOR_BLUE,		/* UPSTREAM */
+	GIT_COLOR_NORMAL,       /* PLAIN */
+	GIT_COLOR_RED,          /* REMOTE */
+	GIT_COLOR_NORMAL,       /* LOCAL */
+	GIT_COLOR_GREEN,        /* CURRENT */
+	GIT_COLOR_BLUE,         /* UPSTREAM */
 };
 enum color_branch {
 	BRANCH_COLOR_RESET = 0,
@@ -280,180 +280,88 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 	return(ret);
 }
 
-static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
-		int show_upstream_ref)
+static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
 {
-	int ours, theirs;
-	char *ref = NULL;
-	struct branch *branch = branch_get(branch_name);
-	const char *upstream;
-	struct strbuf fancy = STRBUF_INIT;
-	int upstream_is_gone = 0;
-	int added_decoration = 1;
-
-	if (stat_tracking_info(branch, &ours, &theirs, &upstream) < 0) {
-		if (!upstream)
-			return;
-		upstream_is_gone = 1;
-	}
-
-	if (show_upstream_ref) {
-		ref = shorten_unambiguous_ref(upstream, 0);
-		if (want_color(branch_use_color))
-			strbuf_addf(&fancy, "%s%s%s",
-					branch_get_color(BRANCH_COLOR_UPSTREAM),
-					ref, branch_get_color(BRANCH_COLOR_RESET));
-		else
-			strbuf_addstr(&fancy, ref);
-	}
+	int i, max = 0;
+	for (i = 0; i < refs->nr; i++) {
+		struct ref_array_item *it = refs->items[i];
+		const char *desc = it->refname;
+		int w;
 
-	if (upstream_is_gone) {
-		if (show_upstream_ref)
-			strbuf_addf(stat, _("[%s: gone]"), fancy.buf);
-		else
-			added_decoration = 0;
-	} else if (!ours && !theirs) {
-		if (show_upstream_ref)
-			strbuf_addf(stat, _("[%s]"), fancy.buf);
-		else
-			added_decoration = 0;
-	} else if (!ours) {
-		if (show_upstream_ref)
-			strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf, theirs);
-		else
-			strbuf_addf(stat, _("[behind %d]"), theirs);
+		skip_prefix(it->refname, "refs/heads/", &desc);
+		skip_prefix(it->refname, "refs/remotes/", &desc);
+		if (it->kind == FILTER_REFS_DETACHED_HEAD) {
+			char *head_desc = get_head_description();
+			w = utf8_strwidth(head_desc);
+			free(head_desc);
+		} else
+			w = utf8_strwidth(desc);
 
-	} else if (!theirs) {
-		if (show_upstream_ref)
-			strbuf_addf(stat, _("[%s: ahead %d]"), fancy.buf, ours);
-		else
-			strbuf_addf(stat, _("[ahead %d]"), ours);
-	} else {
-		if (show_upstream_ref)
-			strbuf_addf(stat, _("[%s: ahead %d, behind %d]"),
-				    fancy.buf, ours, theirs);
-		else
-			strbuf_addf(stat, _("[ahead %d, behind %d]"),
-				    ours, theirs);
+		if (it->kind == FILTER_REFS_REMOTES)
+			w += remote_bonus;
+		if (w > max)
+			max = w;
 	}
-	strbuf_release(&fancy);
-	if (added_decoration)
-		strbuf_addch(stat, ' ');
-	free(ref);
+	return max;
 }
 
-static void add_verbose_info(struct strbuf *out, struct ref_array_item *item,
-			     struct ref_filter *filter, const char *refname)
+static const char *quote_literal_for_format(const char *s)
 {
-	struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
-	const char *sub = _(" **** invalid ref ****");
-	struct commit *commit = item->commit;
+	static struct strbuf buf = STRBUF_INIT;
 
-	if (!parse_commit(commit)) {
-		pp_commit_easy(CMIT_FMT_ONELINE, commit, &subject);
-		sub = subject.buf;
+	strbuf_reset(&buf);
+	while (*s) {
+		const char *ep = strchrnul(s, '%');
+		if (s < ep)
+			strbuf_add(&buf, s, ep - s);
+		if (*ep == '%') {
+			strbuf_addstr(&buf, "%%");
+			s = ep + 1;
+		} else {
+			s = ep;
+		}
 	}
-
-	if (item->kind == FILTER_REFS_BRANCHES)
-		fill_tracking_info(&stat, refname, filter->verbose > 1);
-
-	strbuf_addf(out, " %s %s%s",
-		find_unique_abbrev(item->commit->object.oid.hash, filter->abbrev),
-		stat.buf, sub);
-	strbuf_release(&stat);
-	strbuf_release(&subject);
+	return buf.buf;
 }
 
-static void format_and_print_ref_item(struct ref_array_item *item, int maxwidth,
-				      struct ref_filter *filter, const char *remote_prefix)
+static char *build_format(struct ref_filter *filter, int maxwidth, const char *remote_prefix)
 {
-	char c;
-	int current = 0;
-	int color;
-	struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
-	const char *prefix_to_show = "";
-	const char *prefix_to_skip = NULL;
-	const char *desc = item->refname;
-	char *to_free = NULL;
-
-	switch (item->kind) {
-	case FILTER_REFS_BRANCHES:
-		prefix_to_skip = "refs/heads/";
-		skip_prefix(desc, prefix_to_skip, &desc);
-		if (!filter->detached && !strcmp(desc, head))
-			current = 1;
-		else
-			color = BRANCH_COLOR_LOCAL;
-		break;
-	case FILTER_REFS_REMOTES:
-		prefix_to_skip = "refs/remotes/";
-		skip_prefix(desc, prefix_to_skip, &desc);
-		color = BRANCH_COLOR_REMOTE;
-		prefix_to_show = remote_prefix;
-		break;
-	case FILTER_REFS_DETACHED_HEAD:
-		desc = to_free = get_head_description();
-		current = 1;
-		break;
-	default:
-		color = BRANCH_COLOR_PLAIN;
-		break;
-	}
+	struct strbuf fmt = STRBUF_INIT;
+	struct strbuf local = STRBUF_INIT;
+	struct strbuf remote = STRBUF_INIT;
 
-	c = ' ';
-	if (current) {
-		c = '*';
-		color = BRANCH_COLOR_CURRENT;
-	}
+	strbuf_addf(&fmt, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %%(end)",
+		    branch_get_color(BRANCH_COLOR_CURRENT));
 
-	strbuf_addf(&name, "%s%s", prefix_to_show, desc);
 	if (filter->verbose) {
-		int utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf);
-		strbuf_addf(&out, "%c %s%-*s%s", c, branch_get_color(color),
-			    maxwidth + utf8_compensation, name.buf,
+		strbuf_addf(&local, "%%(align:%d,left)%%(refname:lstrip=2)%%(end)", maxwidth);
+		strbuf_addf(&local, "%s", branch_get_color(BRANCH_COLOR_RESET));
+		strbuf_addf(&local, " %%(objectname:short=7) ");
+
+		if (filter->verbose > 1)
+			strbuf_addf(&local, "%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)"
+				    "%%(then): %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)",
+				    branch_get_color(BRANCH_COLOR_UPSTREAM), branch_get_color(BRANCH_COLOR_RESET));
+		else
+			strbuf_addf(&local, "%%(if)%%(upstream:track)%%(then)%%(upstream:track) %%(end)%%(contents:subject)");
+
+		strbuf_addf(&remote, "%s%%(align:%d,left)%s%%(refname:lstrip=2)%%(end)%s%%(if)%%(symref)%%(then) -> %%(symref:short)"
+			    "%%(else) %%(objectname:short=7) %%(contents:subject)%%(end)",
+			    branch_get_color(BRANCH_COLOR_REMOTE), maxwidth, quote_literal_for_format(remote_prefix),
 			    branch_get_color(BRANCH_COLOR_RESET));
-	} else
-		strbuf_addf(&out, "%c %s%s%s", c, branch_get_color(color),
-			    name.buf, branch_get_color(BRANCH_COLOR_RESET));
-
-	if (item->symref) {
-		const char *symref = item->symref;
-		if (prefix_to_skip)
-			skip_prefix(symref, prefix_to_skip, &symref);
-		strbuf_addf(&out, " -> %s", symref);
-	}
-	else if (filter->verbose)
-		/* " f7c0c00 [ahead 58, behind 197] vcs-svn: drop obj_pool.h" */
-		add_verbose_info(&out, item, filter, desc);
-	if (column_active(colopts)) {
-		assert(!filter->verbose && "--column and --verbose are incompatible");
-		string_list_append(&output, out.buf);
 	} else {
-		printf("%s\n", out.buf);
+		strbuf_addf(&local, "%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
+			    branch_get_color(BRANCH_COLOR_RESET));
+		strbuf_addf(&remote, "%s%s%%(refname:lstrip=2)%s%%(if)%%(symref)%%(then) -> %%(symref:short)%%(end)",
+			    branch_get_color(BRANCH_COLOR_REMOTE), quote_literal_for_format(remote_prefix),
+			    branch_get_color(BRANCH_COLOR_RESET));
 	}
-	strbuf_release(&name);
-	strbuf_release(&out);
-	free(to_free);
-}
-
-static int calc_maxwidth(struct ref_array *refs, int remote_bonus)
-{
-	int i, max = 0;
-	for (i = 0; i < refs->nr; i++) {
-		struct ref_array_item *it = refs->items[i];
-		const char *desc = it->refname;
-		int w;
 
-		skip_prefix(it->refname, "refs/heads/", &desc);
-		skip_prefix(it->refname, "refs/remotes/", &desc);
-		w = utf8_strwidth(desc);
+	strbuf_addf(&fmt, "%%(if:notequals=refs/remotes)%%(refname:rstrip=-2)%%(then)%s%%(else)%s%%(end)", local.buf, remote.buf);
 
-		if (it->kind == FILTER_REFS_REMOTES)
-			w += remote_bonus;
-		if (w > max)
-			max = w;
-	}
-	return max;
+	strbuf_release(&local);
+	strbuf_release(&remote);
+	return strbuf_detach(&fmt, NULL);
 }
 
 static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting)
@@ -462,6 +370,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	struct ref_array array;
 	int maxwidth = 0;
 	const char *remote_prefix = "";
+	struct strbuf out = STRBUF_INIT;
+	char *format;
 
 	/*
 	 * If we are listing more than just remote branches,
@@ -473,18 +383,31 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 
 	memset(&array, 0, sizeof(array));
 
-	verify_ref_format("%(refname)%(symref)");
 	filter_refs(&array, filter, filter->kind | FILTER_REFS_INCLUDE_BROKEN);
 
 	if (filter->verbose)
 		maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
 
+	format = build_format(filter, maxwidth, remote_prefix);
+	verify_ref_format(format);
+
 	ref_array_sort(sorting, &array);
 
-	for (i = 0; i < array.nr; i++)
-		format_and_print_ref_item(array.items[i], maxwidth, filter, remote_prefix);
+	for (i = 0; i < array.nr; i++) {
+		format_ref_array_item(array.items[i], format, 0, &out);
+		if (column_active(colopts)) {
+			assert(!filter->verbose && "--column and --verbose are incompatible");
+			 /* format to a string_list to let print_columns() do its job */
+			string_list_append(&output, out.buf);
+		} else {
+			fwrite(out.buf, 1, out.len, stdout);
+			putchar('\n');
+		}
+		strbuf_release(&out);
+	}
 
 	ref_array_clear(&array);
+	free(format);
 }
 
 static void reject_rebase_or_bisect_branch(const char *target)
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 52283dfc8..45213280a 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -194,7 +194,7 @@ test_expect_success 'local-branch symrefs shortened properly' '
 	git symbolic-ref refs/heads/ref-to-remote refs/remotes/origin/branch-one &&
 	cat >expect <<-\EOF &&
 	  ref-to-branch -> branch-one
-	  ref-to-remote -> refs/remotes/origin/branch-one
+	  ref-to-remote -> origin/branch-one
 	EOF
 	git branch >actual.raw &&
 	grep ref-to <actual.raw >actual &&
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 3d5c238c8..97a07655a 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -44,7 +44,7 @@ b1 [ahead 1, behind 1] d
 b2 [ahead 1, behind 1] d
 b3 [behind 1] b
 b4 [ahead 2] f
-b5 g
+b5 [gone] g
 b6 c
 EOF
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 16/20] ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

Complimenting the existing 'lstrip=<N>' option, add an 'rstrip=<N>'
option which strips `<N>` slash-separated path components from the end
of the refname (e.g., `%(refname:rstrip=2)` turns `refs/tags/foo` into
`refs`).

Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.txt | 38 +++++++++++++++++++----------------
 ref-filter.c                       | 41 ++++++++++++++++++++++++++++++++++++--
 t/t6300-for-each-ref.sh            | 19 ++++++++++++++++++
 3 files changed, 79 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 331e1fef8..08be8462c 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -97,11 +97,13 @@ refname::
 	The option core.warnAmbiguousRefs is used to select the strict
 	abbreviation mode. If `lstrip=<N>` is appended, strips `<N>`
 	slash-separated path components from the front of the refname
-	(e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo`.
+	(e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo` and
+	`%(refname:rstrip=2)` turns `refs/tags/foo` into `refs`).
 	if `<N>` is a negative number, then only `<N>` path components
 	are left behind. (e.g., `%(refname:lstrip=-2)` turns
-	`refs/tags/foo` into `tags/foo`). When the ref does not have
-	enough components, the result becomes an empty string if
+	`refs/tags/foo` into `tags/foo` and `%(refname:rstrip=-1)`
+	turns `refs/tags/foo` into `refs`). When the ref does not
+	have enough components, the result becomes an empty string if
 	stripping with positive <N>, or it becomes the full refname if
 	stripping with negative <N>.  Neither is an error.
 
@@ -120,22 +122,23 @@ objectname::
 
 upstream::
 	The name of a local ref which can be considered ``upstream''
-	from the displayed ref. Respects `:short` and `:lstrip` in the
-	same way as `refname` above.  Additionally respects `:track`
-	to show "[ahead N, behind M]" and `:trackshort` to show the
-	terse version: ">" (ahead), "<" (behind), "<>" (ahead and
-	behind), or "=" (in sync). `:track` also prints "[gone]"
-	whenever unknown upstream ref is encountered. Append
-	`:track,nobracket` to show tracking information without
-	brackets (i.e "ahead N, behind M").  Has no effect if the ref
-	does not have tracking information associated with it.  All
-	the options apart from `nobracket` are mutually exclusive, but
-	if used together the last option is selected.
+	from the displayed ref. Respects `:short`, `:lstrip` and
+	`:rstrip` in the same way as `refname` above.  Additionally
+	respects `:track` to show "[ahead N, behind M]" and
+	`:trackshort` to show the terse version: ">" (ahead), "<"
+	(behind), "<>" (ahead and behind), or "=" (in sync). `:track`
+	also prints "[gone]" whenever unknown upstream ref is
+	encountered. Append `:track,nobracket` to show tracking
+	information without brackets (i.e "ahead N, behind M").  Has
+	no effect if the ref does not have tracking information
+	associated with it.  All the options apart from `nobracket`
+	are mutually exclusive, but if used together the last option
+	is selected.
 
 push::
 	The name of a local ref which represents the `@{push}`
 	location for the displayed ref. Respects `:short`, `:lstrip`,
-	`:track`, and `:trackshort` options as `upstream`
+	`:rstrip`, `:track`, and `:trackshort` options as `upstream`
 	does. Produces an empty string if no `@{push}` ref is
 	configured.
 
@@ -177,8 +180,9 @@ if::
 
 symref::
 	The ref which the given symbolic ref refers to. If not a
-	symbolic ref, nothing is printed. Respects the `:short` and
-	`:lstrip` options in the same way as `refname` above.
+	symbolic ref, nothing is printed. Respects the `:short`,
+	`:lstrip` and `:rstrip` options in the same way as `refname`
+	above.
 
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
diff --git a/ref-filter.c b/ref-filter.c
index 1cd92ea4e..dd7e751f2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -33,8 +33,8 @@ struct if_then_else {
 };
 
 struct refname_atom {
-	enum { R_NORMAL, R_SHORT, R_LSTRIP } option;
-	int lstrip;
+	enum { R_NORMAL, R_SHORT, R_LSTRIP, R_RSTRIP } option;
+	int lstrip, rstrip;
 };
 
 /*
@@ -95,6 +95,10 @@ static void refname_atom_parser_internal(struct refname_atom *atom,
 		atom->option = R_LSTRIP;
 		if (strtol_i(arg, 10, &atom->lstrip))
 			die(_("Integer value expected refname:lstrip=%s"), arg);
+	} else if (skip_prefix(arg, "rstrip=", &arg)) {
+		atom->option = R_RSTRIP;
+		if (strtol_i(arg, 10, &atom->rstrip))
+			die(_("Integer value expected refname:rstrip=%s"), arg);
 	} else
 		die(_("unrecognized %%(%s) argument: %s"), name, arg);
 }
@@ -1125,12 +1129,45 @@ static const char *lstrip_ref_components(const char *refname, int len)
 	return start;
 }
 
+static const char *rstrip_ref_components(const char *refname, int len)
+{
+	long remaining = len;
+	char *start = xstrdup(refname);
+
+	if (len < 0) {
+		int i;
+		const char *p = refname;
+
+		/* Find total no of '/' separated path-components */
+		for (i = 0; p[i]; p[i] == '/' ? i++ : *p++)
+			;
+		/*
+		 * The number of components we need to strip is now
+		 * the total minus the components to be left (Plus one
+		 * because we count the number of '/', but the number
+		 * of components is one more than the no of '/').
+		 */
+		remaining = i + len + 1;
+	}
+
+	while (remaining-- > 0) {
+		char *p = strrchr(start, '/');
+		if (p == NULL)
+			return "";
+		else
+			p[0] = '\0';
+	}
+	return start;
+}
+
 static const char *show_ref(struct refname_atom *atom, const char *refname)
 {
 	if (atom->option == R_SHORT)
 		return shorten_unambiguous_ref(refname, warn_ambiguous_refs);
 	else if (atom->option == R_LSTRIP)
 		return lstrip_ref_components(refname, atom->lstrip);
+	else if (atom->option == R_RSTRIP)
+		return rstrip_ref_components(refname, atom->rstrip);
 	else
 		return refname;
 }
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 203cfaa15..25a9973ce 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -55,14 +55,22 @@ test_atom head refname:lstrip=1 heads/master
 test_atom head refname:lstrip=2 master
 test_atom head refname:lstrip=-1 master
 test_atom head refname:lstrip=-2 heads/master
+test_atom head refname:rstrip=1 refs/heads
+test_atom head refname:rstrip=2 refs
+test_atom head refname:rstrip=-1 refs
+test_atom head refname:rstrip=-2 refs/heads
 test_atom head upstream refs/remotes/origin/master
 test_atom head upstream:short origin/master
 test_atom head upstream:lstrip=2 origin/master
 test_atom head upstream:lstrip=-2 origin/master
+test_atom head upstream:rstrip=2 refs/remotes
+test_atom head upstream:rstrip=-2 refs/remotes
 test_atom head push refs/remotes/myfork/master
 test_atom head push:short myfork/master
 test_atom head push:lstrip=1 remotes/myfork/master
 test_atom head push:lstrip=-1 master
+test_atom head push:rstrip=1 refs/remotes/myfork
+test_atom head push:rstrip=-1 refs
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname $(git rev-parse refs/heads/master)
@@ -631,4 +639,15 @@ test_expect_success 'Verify usage of %(symref:lstrip) atom' '
 	test_cmp expected actual
 '
 
+cat >expected <<EOF
+refs
+refs/heads
+EOF
+
+test_expect_success 'Verify usage of %(symref:rstrip) atom' '
+	git for-each-ref --format="%(symref:rstrip=2)" refs/heads/sym > actual &&
+	git for-each-ref --format="%(symref:rstrip=-2)" refs/heads/sym >> actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 17/20] ref-filter: allow porcelain to translate messages in the output
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

Introduce setup_ref_filter_porcelain_msg() so that the messages used in
the atom %(upstream:track) can be translated if needed. By default, keep
the messages untranslated, which is the right behavior for plumbing
commands. This is needed as we port branch.c to use ref-filter's
printing API's.

Written-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 ref-filter.c | 29 +++++++++++++++++++++++++----
 ref-filter.h |  2 ++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index dd7e751f2..e478ec6c8 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -16,6 +16,27 @@
 #include "trailer.h"
 #include "wt-status.h"
 
+static struct ref_msg {
+	const char *gone;
+	const char *ahead;
+	const char *behind;
+	const char *ahead_behind;
+} msgs = {
+	 /* Untranslated plumbing messages: */
+	"gone",
+	"ahead %d",
+	"behind %d",
+	"ahead %d, behind %d"
+};
+
+void setup_ref_filter_porcelain_msg(void)
+{
+	msgs.gone = _("gone");
+	msgs.ahead = _("ahead %d");
+	msgs.behind = _("behind %d");
+	msgs.ahead_behind = _("ahead %d, behind %d");
+}
+
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 typedef enum { COMPARE_EQUAL, COMPARE_UNEQUAL, COMPARE_NONE } cmp_status;
 
@@ -1181,15 +1202,15 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 	else if (atom->u.remote_ref.option == RR_TRACK) {
 		if (stat_tracking_info(branch, &num_ours,
 				       &num_theirs, NULL)) {
-			*s = xstrdup("gone");
+			*s = xstrdup(msgs.gone);
 		} else if (!num_ours && !num_theirs)
 			*s = "";
 		else if (!num_ours)
-			*s = xstrfmt("behind %d", num_theirs);
+			*s = xstrfmt(msgs.behind, num_theirs);
 		else if (!num_theirs)
-			*s = xstrfmt("ahead %d", num_ours);
+			*s = xstrfmt(msgs.ahead, num_ours);
 		else
-			*s = xstrfmt("ahead %d, behind %d",
+			*s = xstrfmt(msgs.ahead_behind,
 				     num_ours, num_theirs);
 		if (!atom->u.remote_ref.nobracket && *s[0]) {
 			const char *to_free = *s;
diff --git a/ref-filter.h b/ref-filter.h
index 630e7c2b9..44b36eded 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -113,5 +113,7 @@ struct ref_sorting *ref_default_sorting(void);
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);
 /*  Get the current HEAD's description */
 char *get_head_description(void);
+/*  Set up translated strings in the output. */
+void setup_ref_filter_porcelain_msg(void);
 
 #endif /*  REF_FILTER_H  */
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 20/20] branch: implement '--format' option
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

Implement the '--format' option provided by 'ref-filter'. This lets the
user list branches as per desired format similar to the implementation
in 'git for-each-ref'.

Add tests and documentation for the same.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-branch.txt |  7 ++++++-
 builtin/branch.c             | 14 +++++++++-----
 t/t3203-branch-output.sh     | 14 ++++++++++++++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 5516a47b5..1fae4eeee 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[--list] [-v [--abbrev=<length> | --no-abbrev]]
 	[--column[=<options>] | --no-column]
 	[(--merged | --no-merged | --contains) [<commit>]] [--sort=<key>]
-	[--points-at <object>] [<pattern>...]
+	[--points-at <object>] [--format=<format>] [<pattern>...]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
 'git branch' --unset-upstream [<branchname>]
@@ -250,6 +250,11 @@ start-point is either a local or remote-tracking branch.
 --points-at <object>::
 	Only list branches of the given object.
 
+--format <format>::
+	A string that interpolates `%(fieldname)` from the object
+	pointed at by a ref being shown.  The format is the same as
+	that of linkgit:git-for-each-ref[1].
+
 Examples
 --------
 
diff --git a/builtin/branch.c b/builtin/branch.c
index f293ee5b0..cbaa6d03c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -28,6 +28,7 @@ static const char * const builtin_branch_usage[] = {
 	N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
 	N_("git branch [<options>] (-m | -M) [<old-branch>] <new-branch>"),
 	N_("git branch [<options>] [-r | -a] [--points-at]"),
+	N_("git branch [<options>] [-r | -a] [--format]"),
 	NULL
 };
 
@@ -364,14 +365,14 @@ static char *build_format(struct ref_filter *filter, int maxwidth, const char *r
 	return strbuf_detach(&fmt, NULL);
 }
 
-static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting)
+static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
 {
 	int i;
 	struct ref_array array;
 	int maxwidth = 0;
 	const char *remote_prefix = "";
 	struct strbuf out = STRBUF_INIT;
-	char *format;
+	char *to_free = NULL;
 
 	/*
 	 * If we are listing more than just remote branches,
@@ -388,7 +389,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	if (filter->verbose)
 		maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
 
-	format = build_format(filter, maxwidth, remote_prefix);
+	if (!format)
+		format = to_free = build_format(filter, maxwidth, remote_prefix);
 	verify_ref_format(format);
 
 	ref_array_sort(sorting, &array);
@@ -407,7 +409,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	}
 
 	ref_array_clear(&array);
-	free(format);
+	free(to_free);
 }
 
 static void reject_rebase_or_bisect_branch(const char *target)
@@ -528,6 +530,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	struct ref_filter filter;
 	int icase = 0;
 	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
+	const char *format = NULL;
 
 	struct option options[] = {
 		OPT_GROUP(N_("Generic options")),
@@ -569,6 +572,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			N_("print only branches of the object"), 0, parse_opt_object_name
 		},
 		OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
+		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
 		OPT_END(),
 	};
 
@@ -641,7 +645,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		if (!sorting)
 			sorting = ref_default_sorting();
 		sorting->ignore_case = icase;
-		print_ref_list(&filter, sorting);
+		print_ref_list(&filter, sorting, format);
 		print_columns(&output, colopts, NULL);
 		string_list_clear(&output, 0);
 		return 0;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 45213280a..5778c0afe 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -225,4 +225,18 @@ test_expect_success 'sort branches, ignore case' '
 	)
 '
 
+test_expect_success 'git branch --format option' '
+	cat >expect <<-\EOF &&
+	Refname is (HEAD detached from fromtag)
+	Refname is refs/heads/ambiguous
+	Refname is refs/heads/branch-one
+	Refname is refs/heads/branch-two
+	Refname is refs/heads/master
+	Refname is refs/heads/ref-to-branch
+	Refname is refs/heads/ref-to-remote
+	EOF
+	git branch --format="Refname is %(refname)" >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 15/20] ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

Currently the 'lstrip=<N>' option only takes a positive value '<N>'
and strips '<N>' slash-separated path components from the left. Modify
the 'lstrip' option to also take a negative number '<N>' which would
strip from the left as necessary and _leave_ behind only 'N'
slash-separated path components from the right-most end.

For e.g. %(refname:lstrip=-1) would make 'foo/goo/abc' into 'abc'.

Add documentation and tests for the same.

Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.txt |  7 ++++++-
 ref-filter.c                       | 27 ++++++++++++++++++++++-----
 t/t6300-for-each-ref.sh            | 12 ++++++------
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 04ffc3552..331e1fef8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -98,7 +98,12 @@ refname::
 	abbreviation mode. If `lstrip=<N>` is appended, strips `<N>`
 	slash-separated path components from the front of the refname
 	(e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo`.
-	`<N>` must be a positive integer.
+	if `<N>` is a negative number, then only `<N>` path components
+	are left behind. (e.g., `%(refname:lstrip=-2)` turns
+	`refs/tags/foo` into `tags/foo`). When the ref does not have
+	enough components, the result becomes an empty string if
+	stripping with positive <N>, or it becomes the full refname if
+	stripping with negative <N>.  Neither is an error.
 
 objecttype::
 	The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/ref-filter.c b/ref-filter.c
index 76553ebc4..1cd92ea4e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -34,7 +34,7 @@ struct if_then_else {
 
 struct refname_atom {
 	enum { R_NORMAL, R_SHORT, R_LSTRIP } option;
-	unsigned int lstrip;
+	int lstrip;
 };
 
 /*
@@ -93,8 +93,8 @@ static void refname_atom_parser_internal(struct refname_atom *atom,
 		atom->option = R_SHORT;
 	else if (skip_prefix(arg, "lstrip=", &arg)) {
 		atom->option = R_LSTRIP;
-		if (strtoul_ui(arg, 10, &atom->lstrip) || atom->lstrip <= 0)
-			die(_("positive value expected refname:lstrip=%s"), arg);
+		if (strtol_i(arg, 10, &atom->lstrip))
+			die(_("Integer value expected refname:lstrip=%s"), arg);
 	} else
 		die(_("unrecognized %%(%s) argument: %s"), name, arg);
 }
@@ -1091,12 +1091,28 @@ static inline char *copy_advance(char *dst, const char *src)
 	return dst;
 }
 
-static const char *lstrip_ref_components(const char *refname, unsigned int len)
+static const char *lstrip_ref_components(const char *refname, int len)
 {
 	long remaining = len;
 	const char *start = refname;
 
-	while (remaining) {
+	if (len < 0) {
+		int i;
+		const char *p = refname;
+
+		/* Find total no of '/' separated path-components */
+		for (i = 0; p[i]; p[i] == '/' ? i++ : *p++)
+			;
+		/*
+		 * The number of components we need to strip is now
+		 * the total minus the components to be left (Plus one
+		 * because we count the number of '/', but the number
+		 * of components is one more than the no of '/').
+		 */
+		remaining = i + len + 1;
+	}
+
+	while (remaining > 0) {
 		switch (*start++) {
 		case '\0':
 			return "";
@@ -1105,6 +1121,7 @@ static const char *lstrip_ref_components(const char *refname, unsigned int len)
 			break;
 		}
 	}
+
 	return start;
 }
 
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index d3d1a97db..203cfaa15 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -53,12 +53,16 @@ test_atom head refname refs/heads/master
 test_atom head refname:short master
 test_atom head refname:lstrip=1 heads/master
 test_atom head refname:lstrip=2 master
+test_atom head refname:lstrip=-1 master
+test_atom head refname:lstrip=-2 heads/master
 test_atom head upstream refs/remotes/origin/master
 test_atom head upstream:short origin/master
 test_atom head upstream:lstrip=2 origin/master
+test_atom head upstream:lstrip=-2 origin/master
 test_atom head push refs/remotes/myfork/master
 test_atom head push:short myfork/master
 test_atom head push:lstrip=1 remotes/myfork/master
+test_atom head push:lstrip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname $(git rev-parse refs/heads/master)
@@ -141,12 +145,6 @@ test_expect_success 'Check invalid atoms names are errors' '
 	test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
 '
 
-test_expect_success 'arguments to :lstrip must be positive integers' '
-	test_must_fail git for-each-ref --format="%(refname:lstrip=0)" &&
-	test_must_fail git for-each-ref --format="%(refname:lstrip=-1)" &&
-	test_must_fail git for-each-ref --format="%(refname:lstrip=foo)"
-'
-
 test_expect_success 'Check format specifiers are ignored in naming date atoms' '
 	git for-each-ref --format="%(authordate)" refs/heads &&
 	git for-each-ref --format="%(authordate:default) %(authordate)" refs/heads &&
@@ -624,10 +622,12 @@ test_expect_success 'Verify usage of %(symref:short) atom' '
 
 cat >expected <<EOF
 master
+heads/master
 EOF
 
 test_expect_success 'Verify usage of %(symref:lstrip) atom' '
 	git for-each-ref --format="%(symref:lstrip=2)" refs/heads/sym > actual &&
+	git for-each-ref --format="%(symref:lstrip=-2)" refs/heads/sym >> actual &&
 	test_cmp expected actual
 '
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 09/20] ref-filter: make "%(symref)" atom work with the ':short' modifier
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

The "%(symref)" atom doesn't work when used with the ':short' modifier
because we strictly match only 'symref' for setting the 'need_symref'
indicator. Fix this by comparing with the valid_atom rather than the
used_atom.

Add tests for %(symref) and %(symref:short) while we're here.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
 ref-filter.c            |  2 +-
 t/t6300-for-each-ref.sh | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index 6de0927d1..e98ef4bb6 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -352,7 +352,7 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
 		valid_atom[i].parser(&used_atom[at], arg);
 	if (*atom == '*')
 		need_tagged = 1;
-	if (!strcmp(used_atom[at].name, "symref"))
+	if (!strcmp(valid_atom[i].name, "symref"))
 		need_symref = 1;
 	return at;
 }
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index af76dc530..7663a3661 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -38,6 +38,7 @@ test_atom() {
 	case "$1" in
 		head) ref=refs/heads/master ;;
 		 tag) ref=refs/tags/testtag ;;
+		 sym) ref=refs/heads/sym ;;
 		   *) ref=$1 ;;
 	esac
 	printf '%s\n' "$3" >expected
@@ -566,6 +567,7 @@ test_expect_success 'Verify sort with multiple keys' '
 	test_cmp expected actual
 '
 
+
 test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '
 	test_when_finished "git checkout master" &&
 	git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&
@@ -600,4 +602,26 @@ test_expect_success 'basic atom: head contents:trailers' '
 	test_cmp expect actual.clean
 '
 
+test_expect_success 'Add symbolic ref for the following tests' '
+	git symbolic-ref refs/heads/sym refs/heads/master
+'
+
+cat >expected <<EOF
+refs/heads/master
+EOF
+
+test_expect_success 'Verify usage of %(symref) atom' '
+	git for-each-ref --format="%(symref)" refs/heads/sym >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+heads/master
+EOF
+
+test_expect_success 'Verify usage of %(symref:short) atom' '
+	git for-each-ref --format="%(symref:short)" refs/heads/sym >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 11/20] ref-filter: introduce refname_atom_parser()
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

Using refname_atom_parser_internal(), introduce refname_atom_parser()
which will parse the %(symref) and %(refname) atoms. Store the parsed
information into the 'used_atom' structure based on the modifiers used
along with the atoms.

Now the '%(symref)' atom supports the ':strip' atom modifier. Update the
Documentation and tests to reflect this.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.txt |  5 +++
 ref-filter.c                       | 73 +++++++++++++++++++++-----------------
 t/t6300-for-each-ref.sh            |  9 +++++
 3 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 14240b407..5de22cf64 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -170,6 +170,11 @@ if::
 	the value between the %(if:...) and %(then) atoms with the
 	given string.
 
+symref::
+	The ref which the given symbolic ref refers to. If not a
+	symbolic ref, nothing is printed. Respects the `:short` and
+	`:strip` options in the same way as `refname` above.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index c1ebc406c..7bfd65633 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -187,6 +187,11 @@ static void objectname_atom_parser(struct used_atom *atom, const char *arg)
 		die(_("unrecognized %%(objectname) argument: %s"), arg);
 }
 
+static void refname_atom_parser(struct used_atom *atom, const char *arg)
+{
+	return refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
+}
+
 static align_type parse_align_position(const char *s)
 {
 	if (!strcmp(s, "right"))
@@ -257,7 +262,7 @@ static struct {
 	cmp_type cmp_type;
 	void (*parser)(struct used_atom *atom, const char *arg);
 } valid_atom[] = {
-	{ "refname" },
+	{ "refname" , FIELD_STR, refname_atom_parser },
 	{ "objecttype" },
 	{ "objectsize", FIELD_ULONG },
 	{ "objectname", FIELD_STR, objectname_atom_parser },
@@ -287,7 +292,7 @@ static struct {
 	{ "contents", FIELD_STR, contents_atom_parser },
 	{ "upstream", FIELD_STR, remote_ref_atom_parser },
 	{ "push", FIELD_STR, remote_ref_atom_parser },
-	{ "symref" },
+	{ "symref", FIELD_STR, refname_atom_parser },
 	{ "flag" },
 	{ "HEAD" },
 	{ "color", FIELD_STR, color_atom_parser },
@@ -1082,21 +1087,16 @@ static inline char *copy_advance(char *dst, const char *src)
 	return dst;
 }
 
-static const char *strip_ref_components(const char *refname, const char *nr_arg)
+static const char *strip_ref_components(const char *refname, unsigned int len)
 {
-	char *end;
-	long nr = strtol(nr_arg, &end, 10);
-	long remaining = nr;
+	long remaining = len;
 	const char *start = refname;
 
-	if (nr < 1 || *end != '\0')
-		die(_(":strip= requires a positive integer argument"));
-
 	while (remaining) {
 		switch (*start++) {
 		case '\0':
-			die(_("ref '%s' does not have %ld components to :strip"),
-			    refname, nr);
+			die(_("ref '%s' does not have %ud components to :strip"),
+			    refname, len);
 		case '/':
 			remaining--;
 			break;
@@ -1105,6 +1105,16 @@ static const char *strip_ref_components(const char *refname, const char *nr_arg)
 	return start;
 }
 
+static const char *show_ref(struct refname_atom *atom, const char *refname)
+{
+	if (atom->option == R_SHORT)
+		return shorten_unambiguous_ref(refname, warn_ambiguous_refs);
+	else if (atom->option == R_STRIP)
+		return strip_ref_components(refname, atom->strip);
+	else
+		return refname;
+}
+
 static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 				    struct branch *branch, const char **s)
 {
@@ -1177,6 +1187,21 @@ char *get_head_description(void)
 	return strbuf_detach(&desc, NULL);
 }
 
+static const char *get_symref(struct used_atom *atom, struct ref_array_item *ref)
+{
+	if (!ref->symref)
+		return "";
+	else
+		return show_ref(&atom->u.refname, ref->symref);
+}
+
+static const char *get_refname(struct used_atom *atom, struct ref_array_item *ref)
+{
+	if (ref->kind & FILTER_REFS_DETACHED_HEAD)
+		return get_head_description();
+	return show_ref(&atom->u.refname, ref->refname);
+}
+
 /*
  * Parse the object referred by ref, and grab needed value.
  */
@@ -1205,7 +1230,6 @@ static void populate_value(struct ref_array_item *ref)
 		struct atom_value *v = &ref->value[i];
 		int deref = 0;
 		const char *refname;
-		const char *formatp;
 		struct branch *branch = NULL;
 
 		v->handler = append_atom;
@@ -1216,12 +1240,10 @@ static void populate_value(struct ref_array_item *ref)
 			name++;
 		}
 
-		if (starts_with(name, "refname")) {
-			refname = ref->refname;
-			if (ref->kind & FILTER_REFS_DETACHED_HEAD)
-				refname = get_head_description();
-		} else if (starts_with(name, "symref"))
-			refname = ref->symref ? ref->symref : "";
+		if (starts_with(name, "refname"))
+			refname = get_refname(atom, ref);
+		else if (starts_with(name, "symref"))
+			refname = get_symref(atom, ref);
 		else if (starts_with(name, "upstream")) {
 			const char *branch_name;
 			/* only local branches may have an upstream */
@@ -1297,21 +1319,6 @@ static void populate_value(struct ref_array_item *ref)
 		} else
 			continue;
 
-		formatp = strchr(name, ':');
-		if (formatp) {
-			const char *arg;
-
-			formatp++;
-			if (!strcmp(formatp, "short"))
-				refname = shorten_unambiguous_ref(refname,
-						      warn_ambiguous_refs);
-			else if (skip_prefix(formatp, "strip=", &arg))
-				refname = strip_ref_components(refname, arg);
-			else
-				die(_("unknown %.*s format %s"),
-				    (int)(formatp - name), name, formatp);
-		}
-
 		if (!deref)
 			v->s = refname;
 		else
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7663a3661..18a9e2565 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -624,4 +624,13 @@ test_expect_success 'Verify usage of %(symref:short) atom' '
 	test_cmp expected actual
 '
 
+cat >expected <<EOF
+master
+EOF
+
+test_expect_success 'Verify usage of %(symref:strip) atom' '
+	git for-each-ref --format="%(symref:strip=2)" refs/heads/sym > actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 10/20] ref-filter: introduce refname_atom_parser_internal()
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

Since there are multiple atoms which print refs ('%(refname)',
'%(symref)', '%(push)', '%(upstream)'), it makes sense to have a common
ground for parsing them. This would allow us to share implementations of
the atom modifiers between these atoms.

Introduce refname_atom_parser_internal() to act as a common parsing
function for ref printing atoms. This would eventually be used to
introduce refname_atom_parser() and symref_atom_parser() and also be
internally used in remote_ref_atom_parser().

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
 ref-filter.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index e98ef4bb6..c1ebc406c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -32,6 +32,11 @@ struct if_then_else {
 		condition_satisfied : 1;
 };
 
+struct refname_atom {
+	enum { R_NORMAL, R_SHORT, R_STRIP } option;
+	unsigned int strip;
+};
+
 /*
  * An atom is a valid field atom listed below, possibly prefixed with
  * a "*" to denote deref_tag().
@@ -64,6 +69,7 @@ static struct used_atom {
 			enum { O_FULL, O_LENGTH, O_SHORT } option;
 			unsigned int length;
 		} objectname;
+		struct refname_atom refname;
 	} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -77,6 +83,21 @@ static void color_atom_parser(struct used_atom *atom, const char *color_value)
 		die(_("unrecognized color: %%(color:%s)"), color_value);
 }
 
+static void refname_atom_parser_internal(struct refname_atom *atom,
+					 const char *arg, const char *name)
+{
+	if (!arg)
+		atom->option = R_NORMAL;
+	else if (!strcmp(arg, "short"))
+		atom->option = R_SHORT;
+	else if (skip_prefix(arg, "strip=", &arg)) {
+		atom->option = R_STRIP;
+		if (strtoul_ui(arg, 10, &atom->strip) || atom->strip <= 0)
+			die(_("positive value expected refname:strip=%s"), arg);
+	} else
+		die(_("unrecognized %%(%s) argument: %s"), name, arg);
+}
+
 static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 {
 	struct string_list params = STRING_LIST_INIT_DUP;
-- 
2.11.0


^ permalink raw reply related

* [PATCH v10 08/20] ref-filter: add support for %(upstream:track,nobracket)
From: Karthik Nayak @ 2017-01-10  8:49 UTC (permalink / raw)
  To: git; +Cc: jacob.keller, gitster, Karthik Nayak
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>

From: Karthik Nayak <karthik.188@gmail.com>

Add support for %(upstream:track,nobracket) which will print the
tracking information without the brackets (i.e. "ahead N, behind M").
This is needed when we port branch.c to use ref-filter's printing APIs.

Add test and documentation for the same.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.txt | 10 ++++--
 ref-filter.c                       | 67 +++++++++++++++++++++++++-------------
 t/t6300-for-each-ref.sh            |  2 ++
 3 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 94c6b88fa..14240b407 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -120,9 +120,13 @@ upstream::
 	`refname` above.  Additionally respects `:track` to show
 	"[ahead N, behind M]" and `:trackshort` to show the terse
 	version: ">" (ahead), "<" (behind), "<>" (ahead and behind),
-	or "=" (in sync).  Has no effect if the ref does not have
-	tracking information associated with it. `:track` also prints
-	"[gone]" whenever unknown upstream ref is encountered.
+	or "=" (in sync). `:track` also prints "[gone]" whenever
+	unknown upstream ref is encountered. Append `:track,nobracket`
+	to show tracking information without brackets (i.e "ahead N,
+	behind M").  Has no effect if the ref does not have tracking
+	information associated with it.  All the options apart from
+	`nobracket` are mutually exclusive, but if used together the
+	last option is selected.
 
 push::
 	The name of a local ref which represents the `@{push}` location
diff --git a/ref-filter.c b/ref-filter.c
index 998991873..6de0927d1 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -48,8 +48,10 @@ static struct used_atom {
 	union {
 		char color[COLOR_MAXLEN];
 		struct align align;
-		enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
-			remote_ref;
+		struct {
+			enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT } option;
+			unsigned int nobracket : 1;
+		} remote_ref;
 		struct {
 			enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB, C_TRAILERS } option;
 			unsigned int nlines;
@@ -77,16 +79,33 @@ static void color_atom_parser(struct used_atom *atom, const char *color_value)
 
 static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
 {
-	if (!arg)
-		atom->u.remote_ref = RR_NORMAL;
-	else if (!strcmp(arg, "short"))
-		atom->u.remote_ref = RR_SHORTEN;
-	else if (!strcmp(arg, "track"))
-		atom->u.remote_ref = RR_TRACK;
-	else if (!strcmp(arg, "trackshort"))
-		atom->u.remote_ref = RR_TRACKSHORT;
-	else
-		die(_("unrecognized format: %%(%s)"), atom->name);
+	struct string_list params = STRING_LIST_INIT_DUP;
+	int i;
+
+	if (!arg) {
+		atom->u.remote_ref.option = RR_NORMAL;
+		return;
+	}
+
+	atom->u.remote_ref.nobracket = 0;
+	string_list_split(&params, arg, ',', -1);
+
+	for (i = 0; i < params.nr; i++) {
+		const char *s = params.items[i].string;
+
+		if (!strcmp(s, "short"))
+			atom->u.remote_ref.option = RR_SHORTEN;
+		else if (!strcmp(s, "track"))
+			atom->u.remote_ref.option = RR_TRACK;
+		else if (!strcmp(s, "trackshort"))
+			atom->u.remote_ref.option = RR_TRACKSHORT;
+		else if (!strcmp(s, "nobracket"))
+			atom->u.remote_ref.nobracket = 1;
+		else
+			die(_("unrecognized format: %%(%s)"), atom->name);
+	}
+
+	string_list_clear(&params, 0);
 }
 
 static void body_atom_parser(struct used_atom *atom, const char *arg)
@@ -1069,25 +1088,27 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
 				    struct branch *branch, const char **s)
 {
 	int num_ours, num_theirs;
-	if (atom->u.remote_ref == RR_SHORTEN)
+	if (atom->u.remote_ref.option == RR_SHORTEN)
 		*s = shorten_unambiguous_ref(refname, warn_ambiguous_refs);
-	else if (atom->u.remote_ref == RR_TRACK) {
+	else if (atom->u.remote_ref.option == RR_TRACK) {
 		if (stat_tracking_info(branch, &num_ours,
 				       &num_theirs, NULL)) {
-			*s = "[gone]";
-			return;
-		}
-
-		if (!num_ours && !num_theirs)
+			*s = xstrdup("gone");
+		} else if (!num_ours && !num_theirs)
 			*s = "";
 		else if (!num_ours)
-			*s = xstrfmt("[behind %d]", num_theirs);
+			*s = xstrfmt("behind %d", num_theirs);
 		else if (!num_theirs)
-			*s = xstrfmt("[ahead %d]", num_ours);
+			*s = xstrfmt("ahead %d", num_ours);
 		else
-			*s = xstrfmt("[ahead %d, behind %d]",
+			*s = xstrfmt("ahead %d, behind %d",
 				     num_ours, num_theirs);
-	} else if (atom->u.remote_ref == RR_TRACKSHORT) {
+		if (!atom->u.remote_ref.nobracket && *s[0]) {
+			const char *to_free = *s;
+			*s = xstrfmt("[%s]", *s);
+			free((void *)to_free);
+		}
+	} else if (atom->u.remote_ref.option == RR_TRACKSHORT) {
 		if (stat_tracking_info(branch, &num_ours,
 				       &num_theirs, NULL))
 			return;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index a2e3f5525..af76dc530 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -372,6 +372,8 @@ test_expect_success 'setup for upstream:track[short]' '
 
 test_atom head upstream:track '[ahead 1]'
 test_atom head upstream:trackshort '>'
+test_atom head upstream:track,nobracket 'ahead 1'
+test_atom head upstream:nobracket,track 'ahead 1'
 test_atom head push:track '[ahead 1]'
 test_atom head push:trackshort '>'
 
-- 
2.11.0


^ 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