[PATCH] config.c: Expand $HOME and tilde character in core.excludesfile

[PATCH] config.c: Expand $HOME and tilde character in core.excludesfile

[Jari Aalto]

c* str_replace(): New function. Generic replace command.
* str_replace_home(): New funtion. Substitute $HOME and tilde(~) in string.
* git_default_config(): Pass core.excludesfile to str_replace_home().

Signed-off-by: Jari Aalto <jari.aalto AT cante.net>
---

 From ac6941f5055b2acdded59627d228bbf03ba0d9fc

 config.c |   44 +++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 43 insertions(+), 1 deletions(-)

diff --git a/config.c b/config.c
index 526a3f4..7c91689 100644
--- a/config.c
+++ b/config.c
@@ -309,6 +309,46 @@ int git_config_bool(const char *name, const char *value)
 	return git_config_int(name, value) != 0;
 }
 
+char *str_replace(const char *str, const char *find, const char *replace)
+{
+        int maxlen   = strlen(str) + strlen(replace) + 1;
+        char *start  = strstr(str, find);
+        char *ptr    = (char *)malloc(maxlen);
+        int len      = strlen(find);
+        int llen, rlen;         /* left, right portion length */
+
+        if (start == (char *)NULL) {
+                strcpy( ptr, str);
+        }
+        else{
+                rlen = strlen(start) - strlen(find);
+                llen = strlen(str) - strlen(start);
+                strncpy( ptr, str, llen);
+                strcat( ptr, replace);
+                strncat( ptr, start + len, rlen); /* Does not add  '\0' */
+                strcat( ptr, "");   /* Terminate with null string */
+        }
+
+        return ptr;
+}
+
+char *str_replace_home(const char *str)
+{
+        char *ret  = xstrdup(str);
+        char *home = getenv("HOME");
+
+        if (home != (char *)NULL ) {
+                if (strstr(str, "~") != NULL) {
+                        ret = str_replace(str, "~", home);
+                }
+                else if (strstr(str, "$HOME") != NULL) {
+                        ret = str_replace(str, "$HOME", home);
+                }
+        }
+
+        return ret;
+}
+
 int git_default_config(const char *var, const char *value)
 {
 	/* This needs a better name */
@@ -447,7 +487,9 @@ int git_default_config(const char *var, const char *value)
 		if (!value)
 			die("core.excludesfile without value");
 		excludes_file = xstrdup(value);
-		return 0;
+                /* expand $HOME and tilde(~) */
+                excludes_file = str_replace_home(excludes_file);
+                return 0;
 	}
 
 	if (!strcmp(var, "core.whitespace")) {
-- 
1.5.4-rc3.GIT

Re: [PATCH] config.c: Expand $HOME and tilde character in core.excludesfile

[Johannes Schindelin]

Hi,

On Mon, 28 Jan 2008, Jari Aalto wrote:

> c* str_replace(): New function. Generic replace command.
> * str_replace_home(): New funtion. Substitute $HOME and tilde(~) in string.
> * git_default_config(): Pass core.excludesfile to str_replace_home().

I don't like it.  Not only do you fail to provide an example where this 
could be useful, you also introduce a memory leak for every excludes 
setting.

Besides, there is a more fundamental reason to reject this patch: it sets 
a path for an excludes file for everybody, but to a file which usually 
does not exist.

So either the user creates that file, in which case you can expect her to 
adjust ~/.gitconfig, too, or it is not created, in which case the setting 
in /etc/gitconfig is useless.

There is also a third option: there is a file created for every user from 
/etc/skel, but then there can also be a ~/.gitconfig.

Ciao,
Dscho

Re: [PATCH] config.c: Expand $HOME and tilde character in core.excludesfile

[Jakub Narebski]

Jari Aalto <jari.aalto@cante.net> writes:

> * str_replace(): New function. Generic replace command.
> * str_replace_home(): New function. Substitute $HOME and tilde(~) in string.
> * git_default_config(): Pass core.excludesfile to str_replace_home().
> 
> Signed-off-by: Jari Aalto <jari.aalto AT cante.net>

First, git project does NOT use GNU ChangeLog convention for it's
commit messages.  We rather use descriptive commit messages.

Second, I'm not sure about str_replace... how it fits with strbufs?
AFAIK we try to use strbufs whenever possible and feasible, to avoid
errors in git.

Third, I agree that it is a good idea, but I'd rather have *full*
solution, i.e. for git to expand $HOME (or better yet any
environmental variable) and '~' everywhere, not only for
core.excludesfile, but also for --git-dir and GIT_DIR, for
core.worktree and --work-tree and GIT_WORK_TREE, and for all other
config variables and enviroment variables.

> ---
> 
>  From ac6941f5055b2acdded59627d228bbf03ba0d9fc

What does it mean? A bit cryptic, don't you think?


Comments on code below. One thing: we use tabs for indent, not
spaces.  You use spaces in your code, while context uses tabs.

> +char *str_replace(const char *str, const char *find, const char *replace)
> +{
> +        int maxlen   = strlen(str) + strlen(replace) + 1;
> +        char *start  = strstr(str, find);
> +        char *ptr    = (char *)malloc(maxlen);
> +        int len      = strlen(find);
> +        int llen, rlen;         /* left, right portion length */
> +
> +        if (start == (char *)NULL) {

There is no need to cast NULL. Besides, we write IIRC "if (!start)",
it is common enough idiom.

> +                strcpy( ptr, str);

Style: no space after opening parenthesis: "strcpy(ptr, str)".
Performance: I think it would be better to use stpcpy, although I'm
not quite sure if here too.

> +        }
> +        else{
> +                rlen = strlen(start) - strlen(find);
> +                llen = strlen(str) - strlen(start);
> +                strncpy( ptr, str, llen);
> +                strcat( ptr, replace);
> +                strncat( ptr, start + len, rlen); /* Does not add  '\0' */
> +                strcat( ptr, "");   /* Terminate with null string */

Performance suck here; although this is not time-critical path using
sequence of strcat is just bad style. And 'strcat( ptr, "");' just
takes the cake: use "ptr[len] = '\0'", ehere 'len' is calculated
length of string.

P.S. Junio, when do you think 1.5.4 would be finally released? We have
feature freeze still, isn't it?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] config.c: Expand $HOME and tilde character in core.excludesfile

[Wayne Davison]

On Mon, Jan 28, 2008 at 11:49:05PM +0200, Jari Aalto wrote:
> +                        ret = str_replace(str, "~", home);

This would mangle a name with a tilde in it, e.g. "file.c~", etc.

> +                        ret = str_replace(str, "$HOME", home);

This would affect similarly named vars, e.g. $HOMER, etc.

> +                strcat( ptr, "");   /* Terminate with null string */

Calling strcat() requires a null-terminated string.  Instead, assign a
'\0' char at the right position.

..wayne..

Re: [PATCH] config.c: Expand $HOME and tilde character in core.excludesfile

[Miles Bader]

Jakub Narebski <jnareb@gmail.com> writes:
> First, git project does NOT use GNU ChangeLog convention for it's
> commit messages.  We rather use descriptive commit messages.

Not that what Jari wrote had much resemblance to GNU ChangeLog format...

-Miles

-- 
Opposition, n. In politics the party that prevents the Goverment from running
amok by hamstringing it.

Re: [PATCH] config.c: Expand $HOME and tilde character in core.excludesfile

[David Symonds]

On Jan 29, 2008 4:59 PM, Miles Bader <miles.bader@necel.com> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> > First, git project does NOT use GNU ChangeLog convention for it's
> > commit messages.  We rather use descriptive commit messages.
>
> Not that what Jari wrote had much resemblance to GNU ChangeLog format...

What, like http://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs
? It looked like GNU style to me.


Dave.

Re: [PATCH] config.c: Expand $HOME and tilde character in core.excludesfile

[Miles Bader]

On Jan 29, 2008 4:25 PM, David Symonds <dsymonds@gmail.com> wrote:
> > Not that what Jari wrote had much resemblance to GNU ChangeLog format...
>
> What, like http://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs
> ? It looked like GNU style to me.

No, all the details are wrong.  Just about the only thing in common
with ChangeLog format is that he used asterisks as bullets, and a
colon to indicate the descriptive text -- but that's simply what
people tend to do when making an itemized list in ascii...

-Miles

-- 
Do not taunt Happy Fun Ball.