Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files

[Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx>]

nguyenhu@xxxxxxxxxxxxxxx writes:

>>> --- a/dir.c
>>> +++ b/dir.c
>>> @@ -1234,13 +1234,17 @@ int remove_dir_recursively(struct strbuf  
>>> *path, int flag)
>>>  void setup_standard_excludes(struct dir_struct *dir)
>>>  {
>>>  	const char *path;
>>> +	char *xdg_path;
>>>
>>>  	dir->exclude_per_dir = ".gitignore";
>>>  	path = git_path("info/exclude");
>>> +	home_config_paths(NULL, &xdg_path, "ignore");
>>>  	if (!access(path, R_OK))
>>>  		add_excludes_from_file(dir, path);
>>>  	if (excludes_file && !access(excludes_file, R_OK))
>>>  		add_excludes_from_file(dir, excludes_file);
>>> +	else if (!access(xdg_path, R_OK))
>>> +		add_excludes_from_file(dir, xdg_path);
>>>  }
>> Same remark here. Look at the patch I sent earlier to give a default
>> value:
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/133343/focus=133415
>>
>> For example, you version reads from XDG file if core.excludesfile is
>> set, but the file it points to doesn't exist. I don't think this is
>> expected.
>
> Actually, it's the opposite. Our version only read from XDG file if  
> core.excludesfile is not set.

It's what you want to do, but not what I read from the code. Your "else
if" above is reachable if "excludes_file && !access(excludes_file, R_OK)"
is false, which includes the case when excludes_file is set but does not
exist.

Anyway, this just shows that the logic is too complex, we shouldn't need
this discussion with simple enough code.

> echo $HOME and echo "$HOME"
>       both returns /.../t/trash directory.t1306-read-xdg-config-file
> but   echo foo >$HOME   writes in ../t/trash
> while echo foo >"$HOME" writes in t/trash directory.t1306-read-xdg-config-file
> so "$HOME" is needed for the tests to work.

In general, $HOME means "evaluate $HOME and do the whitespace splitting
after", and "$HOME" means "evaluate $HOME and don't do whitespace
splitting". Since "trash directory" contains a space, you need the
quoting.

There are places where quoting is not needed, though, and I think
redirect is part of them. But I wouldn't be surprised if the different
levels of quoting when ran in test-lib broke that.

My advice: always quote variables, unless you have a very good reason
not to do so.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html