* [PATCH] config: allow ~/ and ~user/ in include.path value @ 2012-04-24 11:08 Matthieu Moy 2012-04-24 18:33 ` [PATCH v2] " Matthieu Moy 0 siblings, 1 reply; 6+ messages in thread From: Matthieu Moy @ 2012-04-24 11:08 UTC (permalink / raw) To: git, gitster; +Cc: Matthieu Moy Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> --- config.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index bfe0c79..dd8b9bf 100644 --- a/config.c +++ b/config.c @@ -33,11 +33,14 @@ static const char include_depth_advice[] = "from\n" " %s\n" "Do you have circular includes?"; -static int handle_path_include(const char *path, struct config_include_data *inc) +static int handle_path_include(const char *raw_path, struct config_include_data *inc) { int ret = 0; struct strbuf buf = STRBUF_INIT; - + int must_free_path = 1; + char *path = expand_user_path(raw_path); + if (!path) + return error("Could not expand include path '%s'.", raw_path); /* * Use an absolute path as-is, but interpret relative paths * based on the including config file. @@ -52,6 +55,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc if (slash) strbuf_add(&buf, cf->name, slash - cf->name + 1); strbuf_addstr(&buf, path); + must_free_path = 0; path = buf.buf; } @@ -63,6 +67,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc inc->depth--; } strbuf_release(&buf); + if (must_free_path) + free(path); return ret; } -- 1.7.10.235.gb2edec ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] config: allow ~/ and ~user/ in include.path value 2012-04-24 11:08 [PATCH] config: allow ~/ and ~user/ in include.path value Matthieu Moy @ 2012-04-24 18:33 ` Matthieu Moy 2012-04-25 12:00 ` Jeff King 0 siblings, 1 reply; 6+ messages in thread From: Matthieu Moy @ 2012-04-24 18:33 UTC (permalink / raw) To: git, gitster; +Cc: Matthieu Moy Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> --- Just added a free(path) before must_free_path = 0. Not that the memory leak is really problematic, but let's be clean. config.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index bfe0c79..f054a35 100644 --- a/config.c +++ b/config.c @@ -33,11 +33,14 @@ static const char include_depth_advice[] = "from\n" " %s\n" "Do you have circular includes?"; -static int handle_path_include(const char *path, struct config_include_data *inc) +static int handle_path_include(const char *raw_path, struct config_include_data *inc) { int ret = 0; struct strbuf buf = STRBUF_INIT; - + int must_free_path = 1; + char *path = expand_user_path(raw_path); + if (!path) + return error("Could not expand include path '%s'.", raw_path); /* * Use an absolute path as-is, but interpret relative paths * based on the including config file. @@ -52,6 +55,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc if (slash) strbuf_add(&buf, cf->name, slash - cf->name + 1); strbuf_addstr(&buf, path); + free(path); + must_free_path = 0; path = buf.buf; } @@ -63,6 +68,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc inc->depth--; } strbuf_release(&buf); + if (must_free_path) + free(path); return ret; } -- 1.7.10.235.gb2edec.dirty ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value 2012-04-24 18:33 ` [PATCH v2] " Matthieu Moy @ 2012-04-25 12:00 ` Jeff King 2012-04-25 15:53 ` Matthieu Moy 2012-04-25 20:14 ` Junio C Hamano 0 siblings, 2 replies; 6+ messages in thread From: Jeff King @ 2012-04-25 12:00 UTC (permalink / raw) To: Matthieu Moy; +Cc: git, gitster On Tue, Apr 24, 2012 at 08:33:16PM +0200, Matthieu Moy wrote: > Subject: Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value It seems like such an obvious choice, I'm not sure why I didn't put it in the initial implementation. > config.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) No docs or tests. :P > -static int handle_path_include(const char *path, struct config_include_data *inc) > +static int handle_path_include(const char *raw_path, struct config_include_data *inc) > { > int ret = 0; > struct strbuf buf = STRBUF_INIT; > - > + int must_free_path = 1; > + char *path = expand_user_path(raw_path); > + if (!path) > + return error("Could not expand include path '%s'.", raw_path); > /* > * Use an absolute path as-is, but interpret relative paths > * based on the including config file. > @@ -52,6 +55,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc > if (slash) > strbuf_add(&buf, cf->name, slash - cf->name + 1); > strbuf_addstr(&buf, path); > + free(path); > + must_free_path = 0; > path = buf.buf; > } > > @@ -63,6 +68,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc > inc->depth--; > } > strbuf_release(&buf); > + if (must_free_path) > + free(path); Ugh. This must_free_path business in the middle was quite confusing for me to read (and I think maybe for you to write, since you ended up with a leak in the initial version). If you just detach the strbuf buffer, then must_free_path can go away (you must always free it then). But I think it is even simpler to just keep the allocated bits in a separate variable. Patch is below, with documentation updates and tests. -Peff -- >8 -- Subject: [PATCH] config: expand tildes in include.path variable You can already use relative paths in include.path, which means that including "foo" from your global "~/.gitconfig" will look in your home directory. However, you might want to do something clever like putting "~/.gitconfig-foo" in a specific repository's config file. Signed-off-by: Jeff King <peff@peff.net> --- Documentation/config.txt | 5 ++++- config.c | 6 ++++++ t/t1305-config-include.sh | 8 ++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index fb386ab..0b3f291 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -95,7 +95,9 @@ included file is expanded immediately, as if its contents had been found at the location of the include directive. If the value of the `include.path` variable is a relative path, the path is considered to be relative to the configuration file in which the include directive was -found. See below for examples. +found. The value of `include.path` is subject to tilde expansion: `~/` +is expanded to the value of `$HOME`, and `~user/` to the specified +user's home directory. See below for examples. Example ~~~~~~~ @@ -122,6 +124,7 @@ Example [include] path = /path/to/foo.inc ; include by absolute path path = foo ; expand "foo" relative to the current file + path = ~/foo ; expand "foo" in your $HOME directory Variables ~~~~~~~~~ diff --git a/config.c b/config.c index 68d3294..2bbf02d 100644 --- a/config.c +++ b/config.c @@ -37,6 +37,11 @@ static int handle_path_include(const char *path, struct config_include_data *inc { int ret = 0; struct strbuf buf = STRBUF_INIT; + char *expanded = expand_user_path(path); + + if (!expanded) + return error("Could not expand include path '%s'", path); + path = expanded; /* * Use an absolute path as-is, but interpret relative paths @@ -63,6 +68,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc inc->depth--; } strbuf_release(&buf); + free(expanded); return ret; } diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index 4b1cbaa..a707076 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -29,6 +29,14 @@ test_expect_success 'chained relative paths' ' test_cmp expect actual ' +test_expect_success 'include paths get tilde-expansion' ' + echo "[test]one = 1" >one && + echo "[include]path = ~/one" >.gitconfig && + echo 1 >expect && + git config test.one >actual && + test_cmp expect actual +' + test_expect_success 'include options can still be examined' ' echo "[test]one = 1" >one && echo "[include]path = one" >.gitconfig && -- 1.7.9.6.11.gd9951 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value 2012-04-25 12:00 ` Jeff King @ 2012-04-25 15:53 ` Matthieu Moy 2012-04-25 20:14 ` Junio C Hamano 1 sibling, 0 replies; 6+ messages in thread From: Matthieu Moy @ 2012-04-25 15:53 UTC (permalink / raw) To: Jeff King; +Cc: git, gitster Jeff King <peff@peff.net> writes: > Documentation/config.txt | 5 ++++- > t/t1305-config-include.sh | 8 ++++++++ Nice, thanks. > --- a/config.c > +++ b/config.c > @@ -37,6 +37,11 @@ static int handle_path_include(const char *path, struct config_include_data *inc > { > int ret = 0; > struct strbuf buf = STRBUF_INIT; > + char *expanded = expand_user_path(path); > + > + if (!expanded) > + return error("Could not expand include path '%s'", path); > + path = expanded; > > /* > * Use an absolute path as-is, but interpret relative paths > @@ -63,6 +68,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc > inc->depth--; > } > strbuf_release(&buf); > + free(expanded); Clearly better than my version. Acked-by: Matthieu Moy <Matthieu.Moy@imag.fr> -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value 2012-04-25 12:00 ` Jeff King 2012-04-25 15:53 ` Matthieu Moy @ 2012-04-25 20:14 ` Junio C Hamano 2012-04-26 6:19 ` Jeff King 1 sibling, 1 reply; 6+ messages in thread From: Junio C Hamano @ 2012-04-25 20:14 UTC (permalink / raw) To: Jeff King; +Cc: Matthieu Moy, git Jeff King <peff@peff.net> writes: > diff --git a/Documentation/config.txt b/Documentation/config.txt > index fb386ab..0b3f291 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -95,7 +95,9 @@ included file is expanded immediately, as if its contents had been > found at the location of the include directive. If the value of the > `include.path` variable is a relative path, the path is considered to be > relative to the configuration file in which the include directive was > -found. See below for examples. > +found. The value of `include.path` is subject to tilde expansion: `~/` > +is expanded to the value of `$HOME`, and `~user/` to the specified > +user's home directory. See below for examples. > > Example > ~~~~~~~ > @@ -122,6 +124,7 @@ Example > [include] > path = /path/to/foo.inc ; include by absolute path > path = foo ; expand "foo" relative to the current file > + path = ~/foo ; expand "foo" in your $HOME directory Modulo s/~/{tilde}/ in the body text (but not in the displayed example), looked good to me, so I queued with two amends. Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value 2012-04-25 20:14 ` Junio C Hamano @ 2012-04-26 6:19 ` Jeff King 0 siblings, 0 replies; 6+ messages in thread From: Jeff King @ 2012-04-26 6:19 UTC (permalink / raw) To: Junio C Hamano; +Cc: Matthieu Moy, git On Wed, Apr 25, 2012 at 01:14:32PM -0700, Junio C Hamano wrote: > > +found. The value of `include.path` is subject to tilde expansion: `~/` > > +is expanded to the value of `$HOME`, and `~user/` to the specified > > +user's home directory. See below for examples. > > > > Example > > ~~~~~~~ > > @@ -122,6 +124,7 @@ Example > > [include] > > path = /path/to/foo.inc ; include by absolute path > > path = foo ; expand "foo" relative to the current file > > + path = ~/foo ; expand "foo" in your $HOME directory > > Modulo s/~/{tilde}/ in the body text (but not in the displayed example), > looked good to me, so I queued with two amends. Thanks, I forgot about that. I'd like to eventually stop building the documentation with no-inline-literal. It was an asciidoc7 compatibility thing, but we can probably drop that now. However, I suspect that would require us to simultaneously convert all of the `{tilde}` uses back into `~`. -Peff ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-26 6:19 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-04-24 11:08 [PATCH] config: allow ~/ and ~user/ in include.path value Matthieu Moy 2012-04-24 18:33 ` [PATCH v2] " Matthieu Moy 2012-04-25 12:00 ` Jeff King 2012-04-25 15:53 ` Matthieu Moy 2012-04-25 20:14 ` Junio C Hamano 2012-04-26 6:19 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).