Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@xxxxxxxxxxxxxxx>
writes:
> From: NGUYEN Huynh Khoi Nguyen <nguyenhu@xxxxxxxxxxxxxx>
>
> Git will be able to read in $XDG_CONFIG_HOME/git/config, a new
Getting nicer.
> @@ -172,7 +172,9 @@ static int get_value(const char *key_, const char *regex_)
> const char *home = getenv("HOME");
> local = repo_config = git_pathdup("config");
> if (home)
> - global = xstrdup(mkpath("%s/.gitconfig", home));
> + gitconfig_global = xstrdup(mkpath("%s/.gitconfig", home));
> + if (xdg_git_path("config"))
> + xdg_global = xdg_git_path("config");
> if (git_config_system())
> system_wide = git_etc_gitconfig();
> }
Hrm, xdg_git_path() returns allocated memory, and each call site
leaks its return value, no?
I didn't mean a micro-helper function like xdg_git_path() when I
suggested refactoring. I meant a helper that figures out all the
necessary bits in one go. For example, can't the above call site
look more like this?
static int get_value(const char *key_, const char *regex_)
{
int ret = -1;
char *global = NULL, *xdg = NULL, *repo_config = NULL;
const char *system_wide = NULL, *local;
struct config_include_data inc = CONFIG_INCLUDE_INIT;
config_fn_t fn;
void *data;
local = given_config_file;
if (!local) {
local = repo_config = git_pathdup("config");
if (git_config_system())
system_wide = git_etc_gitconfig();
home_config_paths(&global, &xdg);
}
...
And then the config.c::home_config_paths() may look like:
void home_config_paths(char **global, char **xdg)
{
char *xdg_home = getenv("XDG_CONFIG_HOME");
char *home = getenv("HOME");
char *to_free = NULL;
if (!home) {
*global = NULL;
} else {
if (!xdg_home) {
to_free = strdup(mkpath("%s/.config", home));
xdg_home = to_free;
}
*global = xstrdup(mkpath("%s/.gitconfig", home));
}
if (!xdg_home)
*xdg = NULL;
else
*xdg = xstrdup(mkpath("%s/git/config", xdg_home));
free(to_free);
}
> diff --git a/t/t1306-read-xdg-config-file.sh b/t/t1306-read-xdg-config-file.sh
> new file mode 100755
> index 0000000..4cab38b
> --- /dev/null
> +++ b/t/t1306-read-xdg-config-file.sh
> @@ -0,0 +1,87 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2012 Valentin Duperray, Lucien Kong, Franck Jonas,
> +# Thomas Nguy, Khoi Nguyen
> +# Grenoble INP Ensimag
> +#
> +
> +test_description='possibility to read $XDG_CONFIG_HOME/git/config file'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'read automatically: xdg file exists and ~/.gitconfig doesn'\''t' '
> + mkdir -p .config/git &&
> + echo "[advice]" >.config/git/config &&
> + echo " statushints = false" >>.config/git/config &&
> + cat >expect <<-\EOF &&
> + # On branch master
> + #
> + # Initial commit
Use of "status" output as test vector is not such a good idea for
multiple reasons.
- You are aware that the "status hints" feature is currently in
flux (somebody nearby is working on a patch);
- Even without another topic that currently works on it, "git
status" output is for human consumption and subject to
improvement and also l10n.
- "status" output lists tracked and untracked paths, which makes it
harder to later introduce new file to the test directory that are
needed for later tests, or rename ones that were used in earlier
versions of the test, without having to update all of these test
vectors.
> + #
> + # Untracked files:
> + # .config/
> + # expect
> + # output
This is a good example that shows why using "status" output as a
test vector is not a good idea.
Notice you listed "expect" and "output" here. As my next suggestion
is to rename "output" to "actual" (we typically "test_cmp expect actual"),
use of "git status" output as a test vector would mean you would
have to update them whenever such a change is needed.
> + nothing added to commit but untracked files present
> + EOF
> + git status >output &&
> + test_cmp expect output
> +'
--
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