Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files
From: Matthieu Moy <hidden>
Date: 2016-06-15 22:53:58
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files · Ramsay Jones <hidden>
- 2016-06-15 · Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files · Ramsay Jones <hidden>
- 2016-06-15 · Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files · <hidden>
- 2016-06-15 · [PATCHv4] Read (but not write) from XDG configuration, XDG attributes and XDG ignore files · Huynh Khoi Nguyen NGUYEN <hidden>
Huynh Khoi Nguyen NGUYEN [off-list ref] writes:
Git will not be able to write in this new configuration file.
The use of the future is ambiguous, and since another patch will allow writing, I'd rather rephrase it as e.g. Git currently does not write to this new configuration file.
If core.excludesfile is not define, Git will read the global exclude
s/define/defined/
files in $XDG_CONFIG_HOME/git/ignore. Same goes for core.attributesfile in $XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config will be used.
I'd prefer having two separate patches for the config file and for the two others. If ignore and attributes are simple enough, they may go to the same patch, but ideally, there would be two separate patches again.
quoted hunk
--- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt
No doc for core.excludesfile and core.attributesfile change :-(.
quoted hunk
--- a/attr.c +++ b/attr.c@@ -497,6 +497,9 @@ static int git_attr_system(void) static void bootstrap_attr_stack(void) { struct attr_stack *elem; + char *xdg_attributes_file; + + home_config_paths(NULL, &xdg_attributes_file, "attributes"); if (attr_stack) return;@@ -522,6 +525,13 @@ static void bootstrap_attr_stack(void) elem->prev = attr_stack; attr_stack = elem; } + } else if (!access(xdg_attributes_file, R_OK)) { + elem = read_attr_from_file(xdg_attributes_file, 1); + if (elem) { + elem->origin = NULL; + elem->prev = attr_stack; + attr_stack = elem; + } } if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
The logic seems overly complex, and you duplicate the if() uselessly. Why not just set the variable git_attributes_file before entering the if? Something like this:
diff --git a/attr.c b/attr.c
index 303751f..71dc472 100644
--- a/attr.c
+++ b/attr.c@@ -515,6 +515,9 @@ static void bootstrap_attr_stack(void) } } + if (!git_attributes_file) + git_attributes_file = "foo"; + if (git_attributes_file) { elem = read_attr_from_file(git_attributes_file, 1); if (elem) {
(obviously replacing "foo" by the actual code involving home_config_paths(..., "attributes")). Doing so, you may even get rid of the "if (git_attributes_file)" on the next line. Then, the logic is really "core.excludesfile defaults to the XDG file", not "if such variable is set then do that else do something else".
quoted hunk
--- 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.
+ echo foo >to_be_excluded && + git add to_be_excluded && + git rm --cached to_be_excluded &&
Err, why add and remove it? You just need to create it, right?
+ cd .. && + mkdir -p .config/git/ &&
I don't like these relative references to $HOME. If you mean $HOME, why not say mkdir -p $HOME/.config/git/ echo "f attr_f" >$HOME/.config/git/ ? (you don't even need to "cd") -- Matthieu Moy http://www-verimag.imag.fr/~moy/