All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	sschuberth@gmail.com,
	"Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>,
	"Philip Oakley" <philipoakley@iee.org>,
	"Ramsay Jones" <ramsay@ramsayjones.plus.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v7 0/3] Conditional config include
Date: Wed,  1 Mar 2017 18:26:28 +0700	[thread overview]
Message-ID: <20170301112631.16497-1-pclouds@gmail.com> (raw)
In-Reply-To: <20170224131425.32409-1-pclouds@gmail.com>

v7 changes:

 - typo fix here and there
 - delete a dead TODO comment
 - delete the error on unrecognized condition (we probably want a
   better, general mechanism for troubleshooting config keys anyway)
 - some fix up on include.path document because the "one or many"
   confusion started from there

I don't have a good answer for Jeff's PS about includeIf ugliness. I
agree that includeif is ugly but includeIf looks a bit better. I don't
see a better option (if only "include" does not start or end with a
vowel...). Maybe includewith? Suggestions are welcome.

Interdiff below (just on patch 3, not the first two)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index aab3df04fb..2a41e84bab 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -93,9 +93,11 @@ was found.  See below for examples.
 Conditional includes
 ~~~~~~~~~~~~~~~~~~~~
 
-You can include one config file from another conditionally by setting
-a `includeIf.<condition>.path` variable to the name of the file to be
-included. The variable's value is treated the same way as `include.path`.
+You can include a config file from another conditionally by setting a
+`includeIf.<condition>.path` variable to the name of the file to be
+included. The variable's value is treated the same way as
+`include.path`. `includeIf.<condition>path` supports multiple key
+values.
 
 The condition starts with a keyword followed by a colon and some data
 whose format and meaning depends on the keyword. Supported keywords
@@ -104,13 +106,14 @@ are:
 `gitdir`::
 
 	The data that follows the keyword `gitdir:` is used as a glob
-	pattern. If the location of the .git directory match the
+	pattern. If the location of the .git directory matches the
 	pattern, the include condition is met.
 +
-The .git location which may be auto-discovered, or come from
-`$GIT_DIR` environment variable. If the repository auto discovered via
-a .git file (e.g. from submodules, or a linked worktree), the .git
-location would be the final location, not where the .git file is.
+The .git location may be auto-discovered, or come from `$GIT_DIR`
+environment variable. If the repository is auto discovered via a .git
+file (e.g. from submodules, or a linked worktree), the .git location
+would be the final location where the .git directory is, not where the
+.git file is.
 +
 The pattern can contain standard globbing wildcards and two additional
 ones, `**/` and `/**`, that can match multiple path components. Please
@@ -169,15 +172,15 @@ Example
 		path = ~/foo ; expand "foo" in your `$HOME` directory
 
 	; include if $GIT_DIR is /path/to/foo/.git
-	[include-if "gitdir:/path/to/foo/.git"]
+	[includeIf "gitdir:/path/to/foo/.git"]
 		path = /path/to/foo.inc
 
 	; include for all repositories inside /path/to/group
-	[include-if "gitdir:/path/to/group/"]
+	[includeIf "gitdir:/path/to/group/"]
 		path = /path/to/foo.inc
 
 	; include for all repositories inside $HOME/to/group
-	[include-if "gitdir:~/to/group/"]
+	[includeIf "gitdir:~/to/group/"]
 		path = /path/to/foo.inc
 
 Values
diff --git a/config.c b/config.c
index ad16802c8a..0dac0f4cb2 100644
--- a/config.c
+++ b/config.c
@@ -191,7 +191,6 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
 			return error(_("relative config include "
 				       "conditionals must come from files"));
 
-		/* TODO: escape wildcards */
 		strbuf_add_absolute_path(&path, cf->path);
 		slash = find_last_dir_sep(path.buf);
 		if (!slash)
@@ -245,16 +244,12 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
 
 static int include_condition_is_true(const char *cond, size_t cond_len)
 {
-	/* no condition (i.e., "include.path") is always true */
-	if (!cond)
-		return 1;
 
 	if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len))
 		return include_by_gitdir(cond, cond_len, 0);
 	else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len))
 		return include_by_gitdir(cond, cond_len, 1);
 
-	error(_("unrecognized include condition: %.*s"), (int)cond_len, cond);
 	/* unknown conditionals are always false */
 	return 0;
 }
@@ -278,7 +273,7 @@ int git_config_include(const char *var, const char *value, void *data)
 		ret = handle_path_include(value, inc);
 
 	if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
-	    include_condition_is_true(cond, cond_len) &&
+	    (cond && include_condition_is_true(cond, cond_len)) &&
 	    !strcmp(key, "path"))
 		ret = handle_path_include(value, inc);

Nguyễn Thái Ngọc Duy (3):
  config.txt: clarify multiple key values in include.path
  config.txt: reflow the second include.path paragraph
  config: add conditional include

 Documentation/config.txt  | 77 +++++++++++++++++++++++++++++++++++----
 config.c                  | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++++
 3 files changed, 218 insertions(+), 7 deletions(-)

-- 
2.11.0.157.gd943d85


  parent reply	other threads:[~2017-03-01 11:27 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-23 12:23 [PATCH v5 0/1] Conditional config include Nguyễn Thái Ngọc Duy
2017-02-23 12:23 ` [PATCH v5 1/1] config: add conditional include Nguyễn Thái Ngọc Duy
2017-02-23 19:59   ` Junio C Hamano
2017-02-24  9:37     ` Duy Nguyen
2017-02-24 17:46       ` Junio C Hamano
2017-02-26  6:07         ` Jeff King
2017-02-27 18:42           ` Junio C Hamano
2017-03-06 22:44   ` Stefan Beller
2017-03-07  8:47     ` Jeff King
2017-03-07 18:39       ` Stefan Beller
2017-02-24 13:14 ` [PATCH v6 0/1] Conditional config include Nguyễn Thái Ngọc Duy
2017-02-24 13:14   ` [PATCH v6 1/1] config: add conditional include Nguyễn Thái Ngọc Duy
2017-02-24 18:35     ` Junio C Hamano
2017-02-24 19:48     ` Ramsay Jones
2017-02-24 22:08     ` Philip Oakley
2017-02-26  3:02       ` Duy Nguyen
2017-02-26 12:27         ` Philip Oakley
2017-02-24 13:14   ` [PATCH v6 0/1] Conditional config include Nguyễn Thái Ngọc Duy
2017-02-24 13:18     ` Duy Nguyen
2017-02-24 13:14   ` [PATCH v6 1/1] config: add conditional include Nguyễn Thái Ngọc Duy
2017-02-26  6:12     ` Jeff King
2017-03-01 11:26   ` Nguyễn Thái Ngọc Duy [this message]
2017-03-01 11:26     ` [PATCH v7 1/3] config.txt: clarify multiple key values in include.path Nguyễn Thái Ngọc Duy
2017-03-01 17:40       ` Junio C Hamano
2017-03-01 11:26     ` [PATCH v7 2/3] config.txt: reflow the second include.path paragraph Nguyễn Thái Ngọc Duy
2017-03-01 11:26     ` [PATCH v7 3/3] config: add conditional include Nguyễn Thái Ngọc Duy
2017-03-01 17:16       ` Ramsay Jones
2017-03-01 17:47       ` Junio C Hamano
2017-03-03  6:30         ` Jeff King
2017-03-03 19:05           ` Junio C Hamano
2017-03-03 22:24             ` Jeff King
2017-03-01 17:52     ` [PATCH v7 0/3] Conditional config include Junio C Hamano
2017-03-03  6:33     ` Jeff King
2017-03-03  9:52       ` Duy Nguyen
2017-03-03 19:13         ` Junio C Hamano
2017-03-03 21:08         ` Junio C Hamano
2017-03-03 18:24       ` Junio C Hamano
2017-03-03 22:22         ` Jeff King
2017-03-03 23:22           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170301112631.16497-1-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=philipoakley@iee.org \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sschuberth@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.