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

[PATCH v2 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>
---
Hopefully, all remarks applied. I went for "Git's user configuration", but
I can change it to whatever native speakers think is better.

 builtin/config.c | 31 ++++++++++++++++++++++++++++---
 cache.h          |  1 +
 ident.c          |  2 +-
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..1363478 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(void)
+{
+	struct strbuf buf = STRBUF_INIT;
+	strbuf_addf(&buf,
+		    _("# This is Git's user 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,18 @@ 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);
+				close(fd);
+			}
+			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 fcb511d..b06cbb2 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

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

[Matthieu Moy]

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

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 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

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

[Junio C Hamano]

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

> This allows a caller to request the global config file without requesting
> the XDG one.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---

Will rephrase

    A caller can ask only for XDG location by passing global=NULL.
    Allow it to ask only for the $HOME/.gitconfig by passing
    xdg=NULL.

as you seem to have forgotten ;-)

>  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);
>  }

[PATCH v2 3/3] commit: advertise 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 5ed6036..7502b0e 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"
@@ -1402,6 +1415,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)
 {
@@ -1440,7 +1470,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: [PATCH v2 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>
> ---
> Hopefully, all remarks applied. I went for "Git's user configuration", but
> I can change it to whatever native speakers think is better.

I am not native but I have a slight worry that a template that lists
only core.{user,email} and marked as Git's "user configuration" will
easily mislead the reader that the file is only about these two
(i.e. I am telling the stupid Git who I am), and is not meant to
hold any other configuration that are private to the user.  I still
have funny feeling about "user-wide", but "Git's user configuration"
somehow smells worse for that reason.

What the patch attempts to do is a good idea and the code looks done
right.  Let's queue it and hope people can polish the wording.

Thanks.

>  builtin/config.c | 31 ++++++++++++++++++++++++++++---
>  cache.h          |  1 +
>  ident.c          |  2 +-
>  3 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index fcd8474..1363478 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(void)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	strbuf_addf(&buf,
> +		    _("# This is Git's user 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,18 @@ 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);
> +				close(fd);
> +			}
> +			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 fcb511d..b06cbb2 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);

[PATCH v3 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>
---
> I am not native but I have a slight worry that a template that lists
> only core.{user,email} and marked as Git's "user configuration" will
> easily mislead the reader that the file is only about these two

Good point, here's the one with "per-user", and PATCH 2 with your
improved commit message.

 builtin/config.c | 31 ++++++++++++++++++++++++++++---
 cache.h          |  1 +
 ident.c          |  2 +-
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index fcd8474..aba7135 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(void)
+{
+	struct strbuf buf = STRBUF_INIT;
+	strbuf_addf(&buf,
+		    _("# This is Git's per-user 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,18 @@ 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);
+				close(fd);
+			}
+			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 fcb511d..b06cbb2 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

[PATCH v3 2/3] home_config_paths(): let the caller ignore xdg path

[Matthieu Moy]

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 path 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.

Commit-message-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 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

[PATCH v3 3/3] commit: advertise 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 5ed6036..7502b0e 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"
@@ -1402,6 +1415,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)
 {
@@ -1440,7 +1470,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: [PATCH v3 3/3] commit: advertise config --global --edit on guessed identity

[Junio C Hamano]

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

> +static const char *implicit_ident_advice() {

Style:

static const char *implicit_ident_advice(void)
{

No need to resend.

Thanks.

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

[Junio C Hamano]

Thanks; will queue the patches from this iteration.