[RFC PATCH 1/3] config --global --edit: create a template file if needed

[RFC PATCH 1/3] config --global --edit: create a template file if needed

[Matthieu Moy]

When the user has no ~/.gitconfig file, git config --global --edit used
to launch an editor on an nonexistant file name.

Instead, create a file with a default content before launching the
editor. The template contains only commented-out entries, to save a few
keystrokes for the user. If the values are guessed properly, the user
will only have to uncomment the entries.

Advanced users teaching newbies can create a minimalistic configuration
faster for newbies. Beginners reading a tutorial advising to run "git
config --global --edit" as a first step will be slightly more guided for
their first contact with Git.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/config.c | 30 +++++++++++++++++++++++++++---
 cache.h          |  1 +
 ident.c          |  2 +-
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..3821697 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
 	return 0;
 }
 
+static char *default_user_config()
+{
+	struct strbuf buf = STRBUF_INIT;
+	strbuf_addf(&buf,
+		    _("# This is Git's user-wide configuration file.\n"
+		      "[core]\n"
+		      "# Please, adapt and uncomment the following lines:\n"
+		      "#	user = %s\n"
+		      "#	email = %s\n"),
+		    ident_default_name(),
+		    ident_default_email());
+	return strbuf_detach(&buf, NULL);
+}
+
 int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	int nongit = !startup_info->have_repository;
@@ -551,6 +565,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		}
 	}
 	else if (actions == ACTION_EDIT) {
+		const char *config_file = given_config_source.file ?
+			given_config_source.file : git_path("config");
 		check_argc(argc, 0, 0);
 		if (!given_config_source.file && nongit)
 			die("not in a git directory");
@@ -559,9 +575,17 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		if (given_config_source.blob)
 			die("editing blobs is not supported");
 		git_config(git_default_config, NULL);
-		launch_editor(given_config_source.file ?
-			      given_config_source.file : git_path("config"),
-			      NULL, NULL);
+		if (use_global_config) {
+			int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY, 0666);
+			if (fd) {
+				char *content = default_user_config();
+				write_str_in_full(fd, content);
+				free(content);
+			}
+			else if (errno != EEXIST)
+				die_errno(_("Cannot create configuration file %s"), config_file);
+		}
+		launch_editor(config_file, NULL, NULL);
 	}
 	else if (actions == ACTION_SET) {
 		int ret;
diff --git a/cache.h b/cache.h
index 2f63fd1..8c79c0c 100644
--- a/cache.h
+++ b/cache.h
@@ -1061,6 +1061,7 @@ extern const char *git_author_info(int);
 extern const char *git_committer_info(int);
 extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
 extern const char *fmt_name(const char *name, const char *email);
+extern const char *ident_default_name(void);
 extern const char *ident_default_email(void);
 extern const char *git_editor(void);
 extern const char *git_pager(int stdout_is_tty);
diff --git a/ident.c b/ident.c
index 1d9b6e7..77bc882 100644
--- a/ident.c
+++ b/ident.c
@@ -102,7 +102,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email)
 	add_domainname(email);
 }
 
-static const char *ident_default_name(void)
+const char *ident_default_name(void)
 {
 	if (!git_default_name.len) {
 		copy_gecos(xgetpwuid_self(), &git_default_name);
-- 
2.0.2.737.gfb43bde

[RFC PATCH 2/3] home_config_path: allow NULL xdg parameter

[Matthieu Moy]

This allows a caller to requst the global config file without requesting
the XDG one.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
This is actually not needed, but I wrote this for a previous version,
and it seems sensible anyway.

 path.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/path.c b/path.c
index 3afcdb4..f68df0c 100644
--- a/path.c
+++ b/path.c
@@ -148,10 +148,12 @@ void home_config_paths(char **global, char **xdg, char *file)
 			*global = mkpathdup("%s/.gitconfig", home);
 	}
 
-	if (!xdg_home)
-		*xdg = NULL;
-	else
-		*xdg = mkpathdup("%s/git/%s", xdg_home, file);
+	if (xdg) {
+		if (!xdg_home)
+			*xdg = NULL;
+		else
+			*xdg = mkpathdup("%s/git/%s", xdg_home, file);
+	}
 
 	free(to_free);
 }
-- 
2.0.2.737.gfb43bde

[RFC PATCH 3/3] commit: advertize config --global --edit on guessed identity

[Matthieu Moy]

When the user has no user-wide configuration file, it's faster to use the
newly introduced config file template than to run two commands to set
user.name and user.email. Advise this to the user.

The old advice is kept if the user already has a configuration file since
the template feature would not trigger in this case.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/commit.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index f2d7979..52ef5a8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -42,7 +42,20 @@ static const char * const builtin_status_usage[] = {
 	NULL
 };
 
-static const char implicit_ident_advice[] =
+static const char implicit_ident_advice_noconfig[] =
+N_("Your name and email address were configured automatically based\n"
+"on your username and hostname. Please check that they are accurate.\n"
+"You can suppress this message by setting them explicitly. Run the\n"
+"following command and follow the instructions in your editor to edit\n"
+"your configuration file:\n"
+"\n"
+"    git config --global --edit\n"
+"\n"
+"After doing this, you may fix the identity used for this commit with:\n"
+"\n"
+"    git commit --amend --reset-author\n");
+
+static const char implicit_ident_advice_config[] =
 N_("Your name and email address were configured automatically based\n"
 "on your username and hostname. Please check that they are accurate.\n"
 "You can suppress this message by setting them explicitly:\n"
@@ -1403,6 +1416,23 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
+static const char *implicit_ident_advice() {
+	char *user_config = NULL;
+	char *xdg_config = NULL;
+	int config_exists;
+
+	home_config_paths(&user_config, &xdg_config, "config");
+	config_exists = file_exists(user_config) || file_exists(xdg_config);
+	free(user_config);
+	free(xdg_config);
+
+	if (config_exists)
+		return _(implicit_ident_advice_config);
+	else
+		return _(implicit_ident_advice_noconfig);
+
+}
+
 static void print_summary(const char *prefix, const unsigned char *sha1,
 			  int initial_commit)
 {
@@ -1441,7 +1471,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
 		strbuf_addbuf_percentquote(&format, &committer_ident);
 		if (advice_implicit_identity) {
 			strbuf_addch(&format, '\n');
-			strbuf_addstr(&format, _(implicit_ident_advice));
+			strbuf_addstr(&format, implicit_ident_advice());
 		}
 	}
 	strbuf_release(&author_ident);
-- 
2.0.2.737.gfb43bde

Re: [RFC PATCH 2/3] home_config_path: allow NULL xdg parameter

[Tanay Abhra]

On 7/25/2014 7:14 PM, Matthieu Moy wrote:
> This allows a caller to requst the global config file without requesting
nit s/requst/request/

Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed

[Eric Sunshine]

On Fri, Jul 25, 2014 at 9:44 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> When the user has no ~/.gitconfig file, git config --global --edit used
> to launch an editor on an nonexistant file name.
>
> Instead, create a file with a default content before launching the
> editor. The template contains only commented-out entries, to save a few
> keystrokes for the user. If the values are guessed properly, the user
> will only have to uncomment the entries.
>
> Advanced users teaching newbies can create a minimalistic configuration
> faster for newbies. Beginners reading a tutorial advising to run "git
> config --global --edit" as a first step will be slightly more guided for
> their first contact with Git.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  builtin/config.c | 30 +++++++++++++++++++++++++++---
>  cache.h          |  1 +
>  ident.c          |  2 +-
>  3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index fcd8474..3821697 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
>         return 0;
>  }
>
> +static char *default_user_config()
> +{
> +       struct strbuf buf = STRBUF_INIT;
> +       strbuf_addf(&buf,
> +                   _("# This is Git's user-wide configuration file.\n"
> +                     "[core]\n"
> +                     "# Please, adapt and uncomment the following lines:\n"
> +                     "#        user = %s\n"
> +                     "#        email = %s\n"),

"[core]", "user =", "email =" should not be translated. Would it make
sense to keep these outside of _()?

> +                   ident_default_name(),
> +                   ident_default_email());
> +       return strbuf_detach(&buf, NULL);
> +}
> +
>  int cmd_config(int argc, const char **argv, const char *prefix)
>  {
>         int nongit = !startup_info->have_repository;
> @@ -551,6 +565,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>                 }
>         }
>         else if (actions == ACTION_EDIT) {
> +               const char *config_file = given_config_source.file ?
> +                       given_config_source.file : git_path("config");
>                 check_argc(argc, 0, 0);
>                 if (!given_config_source.file && nongit)
>                         die("not in a git directory");
> @@ -559,9 +575,17 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>                 if (given_config_source.blob)
>                         die("editing blobs is not supported");
>                 git_config(git_default_config, NULL);
> -               launch_editor(given_config_source.file ?
> -                             given_config_source.file : git_path("config"),
> -                             NULL, NULL);
> +               if (use_global_config) {
> +                       int fd = open(config_file, O_CREAT | O_EXCL | O_WRONLY, 0666);
> +                       if (fd) {
> +                               char *content = default_user_config();
> +                               write_str_in_full(fd, content);

close(fd);

> +                               free(content);
> +                       }
> +                       else if (errno != EEXIST)
> +                               die_errno(_("Cannot create configuration file %s"), config_file);

Other error messages in this file (including those just above this
block) begin with a lowercase letter.

> +               }
> +               launch_editor(config_file, NULL, NULL);
>         }
>         else if (actions == ACTION_SET) {
>                 int ret;
> diff --git a/cache.h b/cache.h
> index 2f63fd1..8c79c0c 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1061,6 +1061,7 @@ extern const char *git_author_info(int);
>  extern const char *git_committer_info(int);
>  extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
>  extern const char *fmt_name(const char *name, const char *email);
> +extern const char *ident_default_name(void);
>  extern const char *ident_default_email(void);
>  extern const char *git_editor(void);
>  extern const char *git_pager(int stdout_is_tty);
> diff --git a/ident.c b/ident.c
> index 1d9b6e7..77bc882 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -102,7 +102,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email)
>         add_domainname(email);
>  }
>
> -static const char *ident_default_name(void)
> +const char *ident_default_name(void)
>  {
>         if (!git_default_name.len) {
>                 copy_gecos(xgetpwuid_self(), &git_default_name);
> --
> 2.0.2.737.gfb43bde

Re: [RFC PATCH 3/3] commit: advertize config --global --edit on guessed identity

[Eric Sunshine]

On Fri, Jul 25, 2014 at 9:44 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> commit: advertize config --global --edit on guessed identity

s/advertize/advertise/

> When the user has no user-wide configuration file, it's faster to use the
> newly introduced config file template than to run two commands to set
> user.name and user.email. Advise this to the user.
>
> The old advice is kept if the user already has a configuration file since
> the template feature would not trigger in this case.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  builtin/commit.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index f2d7979..52ef5a8 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -42,7 +42,20 @@ static const char * const builtin_status_usage[] = {
>         NULL
>  };
>
> -static const char implicit_ident_advice[] =
> +static const char implicit_ident_advice_noconfig[] =
> +N_("Your name and email address were configured automatically based\n"
> +"on your username and hostname. Please check that they are accurate.\n"
> +"You can suppress this message by setting them explicitly. Run the\n"
> +"following command and follow the instructions in your editor to edit\n"
> +"your configuration file:\n"
> +"\n"
> +"    git config --global --edit\n"
> +"\n"
> +"After doing this, you may fix the identity used for this commit with:\n"
> +"\n"
> +"    git commit --amend --reset-author\n");
> +
> +static const char implicit_ident_advice_config[] =
>  N_("Your name and email address were configured automatically based\n"
>  "on your username and hostname. Please check that they are accurate.\n"
>  "You can suppress this message by setting them explicitly:\n"
> @@ -1403,6 +1416,23 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>         return 0;
>  }
>
> +static const char *implicit_ident_advice() {
> +       char *user_config = NULL;
> +       char *xdg_config = NULL;
> +       int config_exists;
> +
> +       home_config_paths(&user_config, &xdg_config, "config");
> +       config_exists = file_exists(user_config) || file_exists(xdg_config);
> +       free(user_config);
> +       free(xdg_config);
> +
> +       if (config_exists)
> +               return _(implicit_ident_advice_config);
> +       else
> +               return _(implicit_ident_advice_noconfig);
> +
> +}
> +
>  static void print_summary(const char *prefix, const unsigned char *sha1,
>                           int initial_commit)
>  {
> @@ -1441,7 +1471,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
>                 strbuf_addbuf_percentquote(&format, &committer_ident);
>                 if (advice_implicit_identity) {
>                         strbuf_addch(&format, '\n');
> -                       strbuf_addstr(&format, _(implicit_ident_advice));
> +                       strbuf_addstr(&format, implicit_ident_advice());
>                 }
>         }
>         strbuf_release(&author_ident);
> --
> 2.0.2.737.gfb43bde

Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed

[Matthieu Moy]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +static char *default_user_config()
>> +{
>> +       struct strbuf buf = STRBUF_INIT;
>> +       strbuf_addf(&buf,
>> +                   _("# This is Git's user-wide configuration file.\n"
>> +                     "[core]\n"
>> +                     "# Please, adapt and uncomment the following lines:\n"
>> +                     "#        user = %s\n"
>> +                     "#        email = %s\n"),
>
> "[core]", "user =", "email =" should not be translated. Would it make
> sense to keep these outside of _()?

I would say no, as the code and the string to translate would be much
less readable without core, user and email inline.

Were you suggesting stg like

_("# This is Git's user-wide configuration file.\n"
  "[%s]\n"
  "# Please, adapt and uncomment the following lines:\n"
  "#        %s = %s\n"
  "#        %s = %s\n"),
  "core", "name", ..., "email", ...

?

>> +                       if (fd) {
>> +                               char *content = default_user_config();
>> +                               write_str_in_full(fd, content);
>
> close(fd);

Indeed.

>> +                               free(content);
>> +                       }
>> +                       else if (errno != EEXIST)
>> +                               die_errno(_("Cannot create configuration file %s"), config_file);
>
> Other error messages in this file (including those just above this
> block) begin with a lowercase letter.

Applied.

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> When the user has no ~/.gitconfig file, git config --global --edit used
> to launch an editor on an nonexistant file name.
>
> Instead, create a file with a default content before launching the
> editor. The template contains only commented-out entries, to save a few
> keystrokes for the user. If the values are guessed properly, the user
> will only have to uncomment the entries.
>
> Advanced users teaching newbies can create a minimalistic configuration
> faster for newbies. Beginners reading a tutorial advising to run "git
> config --global --edit" as a first step will be slightly more guided for
> their first contact with Git.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---

Probably a good idea; I do not think of any possible interactions we
have to worry about with the configuration file init-db creates with
possible templating.

Do we use "user-wide" as a phrase to refer to these?  It sounds
somewhat funny to call anything specific to $frotz "$frotz-wide", at
least to me.

Surely, /etc/gitconfig is called "site-wide".  But .git/config is
per-project (or project-specific), and I would always have thought
that ~/.gitconfig was "per-user".

>  builtin/config.c | 30 +++++++++++++++++++++++++++---
>  cache.h          |  1 +
>  ident.c          |  2 +-
>  3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index fcd8474..3821697 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
>  	return 0;
>  }
>  
> +static char *default_user_config()

static char *default_user_config(void)

> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	strbuf_addf(&buf,
> +		    _("# This is Git's user-wide configuration file.\n"
> +		      "[core]\n"
> +		      "# Please, adapt and uncomment the following lines:\n"

tangent: is it a French tradition to always have comma after please?

Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed

[Eric Sunshine]

On Fri, Jul 25, 2014 at 12:01 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>>> +static char *default_user_config()
>>> +{
>>> +       struct strbuf buf = STRBUF_INIT;
>>> +       strbuf_addf(&buf,
>>> +                   _("# This is Git's user-wide configuration file.\n"
>>> +                     "[core]\n"
>>> +                     "# Please, adapt and uncomment the following lines:\n"
>>> +                     "#        user = %s\n"
>>> +                     "#        email = %s\n"),
>>
>> "[core]", "user =", "email =" should not be translated. Would it make
>> sense to keep these outside of _()?
>
> I would say no, as the code and the string to translate would be much
> less readable without core, user and email inline.
>
> Were you suggesting stg like
>
> _("# This is Git's user-wide configuration file.\n"
>   "[%s]\n"
>   "# Please, adapt and uncomment the following lines:\n"
>   "#        %s = %s\n"
>   "#        %s = %s\n"),
>   "core", "name", ..., "email", ...
>
> ?

That or some equivalent variation. I'm not a translator, but the above
seems to convey sufficient context for a translator to understand what
needs to be said, while preventing accidental translations of those
strings which should not be translated.

>>> +                       if (fd) {
>>> +                               char *content = default_user_config();
>>> +                               write_str_in_full(fd, content);
>>
>> close(fd);
>
> Indeed.
>
>>> +                               free(content);
>>> +                       }
>>> +                       else if (errno != EEXIST)
>>> +                               die_errno(_("Cannot create configuration file %s"), config_file);
>>
>> Other error messages in this file (including those just above this
>> block) begin with a lowercase letter.
>
> Applied.
>
> Thanks,
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/

Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Probably a good idea; I do not think of any possible interactions we
> have to worry about with the configuration file init-db creates with
> possible templating.

The feature should trigger only for --global, so it shouldn't interfer
with .git/config and templates.

> Do we use "user-wide" as a phrase to refer to these?  It sounds
> somewhat funny to call anything specific to $frotz "$frotz-wide", at
> least to me.
>
> Surely, /etc/gitconfig is called "site-wide".  But .git/config is
> per-project (or project-specific), and I would always have thought
> that ~/.gitconfig was "per-user".

I'm not a native speaker, but to me, "user-wide" insists on the fact
that it applies to everything for this user, and "per-user" insists on
the fact that it does not apply to other users.

Perhaps just "Git's user configuration file" would be enough.

>>  builtin/config.c | 30 +++++++++++++++++++++++++++---
>>  cache.h          |  1 +
>>  ident.c          |  2 +-
>>  3 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/builtin/config.c b/builtin/config.c
>> index fcd8474..3821697 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -445,6 +445,20 @@ static int get_urlmatch(const char *var, const char *url)
>>  	return 0;
>>  }
>>  
>> +static char *default_user_config()
>
> static char *default_user_config(void)

Right. Doing too much C++.

>> +{
>> +	struct strbuf buf = STRBUF_INIT;
>> +	strbuf_addf(&buf,
>> +		    _("# This is Git's user-wide configuration file.\n"
>> +		      "[core]\n"
>> +		      "# Please, adapt and uncomment the following lines:\n"
>
> tangent: is it a French tradition to always have comma after please?

Perhaps. In French, the comma would be required after "S'il vous plait"
(litterally, "if you like"). I'll remove it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC PATCH 1/3] config --global --edit: create a template file if needed

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>>> +static char *default_user_config()
>>> +{
>>> +       struct strbuf buf = STRBUF_INIT;
>>> +       strbuf_addf(&buf,
>>> +                   _("# This is Git's user-wide configuration file.\n"
>>> +                     "[core]\n"
>>> +                     "# Please, adapt and uncomment the following lines:\n"
>>> +                     "#        user = %s\n"
>>> +                     "#        email = %s\n"),
>>
>> "[core]", "user =", "email =" should not be translated. Would it make
>> sense to keep these outside of _()?
>
> I would say no, as the code and the string to translate would be much
> less readable without core, user and email inline.
>
> Were you suggesting stg like
>
> _("# This is Git's user-wide configuration file.\n"
>   "[%s]\n"
>   "# Please, adapt and uncomment the following lines:\n"
>   "#        %s = %s\n"
>   "#        %s = %s\n"),
>   "core", "name", ..., "email", ...
>
> ?

;-) That is a clever way to say what my first reaction to Eric's
comment was, which was to have this as multiple strbuf_addf().

Technically speaking, the '#' at the beginning of lines must not be
translated, either, and if that goes without saying, i.e. if the
translators know well enough not to change them, then I can be
persuaded that we can expect that translators know well enough not
to touch the three substrings Eric pointed out.

So, the original message may be fine as-is.

>>> +                       if (fd) {
>>> +                               char *content = default_user_config();
>>> +                               write_str_in_full(fd, content);
>>
>> close(fd);
>
> Indeed.
>
>>> +                               free(content);
>>> +                       }
>>> +                       else if (errno != EEXIST)
>>> +                               die_errno(_("Cannot create configuration file %s"), config_file);
>>
>> Other error messages in this file (including those just above this
>> block) begin with a lowercase letter.
>
> Applied.
>
> Thanks,

Re: [RFC PATCH 2/3] home_config_path: allow NULL xdg parameter

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> This allows a caller to requst the global config file without requesting
> the XDG one.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> This is actually not needed, but I wrote this for a previous version,
> and it seems sensible anyway.

I was about to say "Let's not do this until some caller needs it",
implicitly assuming that we do not let global=NULL to signal that
the caller is interested only in XDG, but I checked and we do check
the NULL-ness of global, so it is consistent to do so for xdg.  The
change makes very good sense.

I wouldn't have had to spend the time to dig, if the log message
justified it that way, instead of having "actually not needed"
comment there ;-)

	home_config_paths(): let the caller ignore xdg path

	The caller can signal that it is not interested in learning
	the location of $HOME/.gitconfig by passing global=NULL, but
	there is no way to decline the ptah to the configuration
	file based on $XDG_CONFIG_HOME.

        Allow the caller to pass xdg=NULL to signal that it is not
        interested in the XDG location.

or something, perhaps?

>
>  path.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/path.c b/path.c
> index 3afcdb4..f68df0c 100644
> --- a/path.c
> +++ b/path.c
> @@ -148,10 +148,12 @@ void home_config_paths(char **global, char **xdg, char *file)
>  			*global = mkpathdup("%s/.gitconfig", home);
>  	}
>  
> -	if (!xdg_home)
> -		*xdg = NULL;
> -	else
> -		*xdg = mkpathdup("%s/git/%s", xdg_home, file);
> +	if (xdg) {
> +		if (!xdg_home)
> +			*xdg = NULL;
> +		else
> +			*xdg = mkpathdup("%s/git/%s", xdg_home, file);
> +	}
>  
>  	free(to_free);
>  }

Re: [RFC PATCH 2/3] home_config_path: allow NULL xdg parameter

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> This allows a caller to requst the global config file without requesting
>> the XDG one.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>> This is actually not needed, but I wrote this for a previous version,
>> and it seems sensible anyway.
>
> I was about to say "Let's not do this until some caller needs it",
> implicitly assuming that we do not let global=NULL to signal that
> the caller is interested only in XDG, but I checked and we do check
> the NULL-ness of global, so it is consistent to do so for xdg.  The
> change makes very good sense.
>
> I wouldn't have had to spend the time to dig, if the log message
> justified it that way, instead of having "actually not needed"
> comment there ;-)
>
> 	home_config_paths(): let the caller ignore xdg path
>
> 	The caller can signal that it is not interested in learning
> 	the location of $HOME/.gitconfig by passing global=NULL, but
> 	there is no way to decline the ptah to the configuration
> 	file based on $XDG_CONFIG_HOME.
>
>         Allow the caller to pass xdg=NULL to signal that it is not
>         interested in the XDG location.

Good point. Applied locally, I'll resend later.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/