threads / patch / 37215

v2, 3 partsconfig --global --edit: create a template file if needed

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

## tl;dr

10 messages between Jul 25, 2014 and Jul 25, 2014. Diffs are folded; open one to read it.

replies: 9people: 2as markdown or json

Matthieu Moy· Jul 25, 2014, 18:00 UTC · lore

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(-)
Show changes to 3 files +30 −4

builtin/config.c, cache.h, ident.c

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
Matthieu Moy· Jul 25, 2014, 18:00 UTC · re: Matthieu Moy · lore

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

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(-)
Show changes to path.c +6 −4
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
Junio C Hamano· Jul 25, 2014, 19:09 UTC · re: Matthieu Moy · lore

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

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 5 quoted lines
> 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 ;-)
Show 24 quoted lines
>  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);
>  }
Matthieu Moy· Jul 25, 2014, 18:00 UTC · re: Matthieu Moy · lore

[PATCH v2 3/3] commit: advertise config --global --edit on guessed identity

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(-)
Show changes to builtin/commit.c +32 −2
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
Junio C Hamano· Jul 25, 2014, 19:05 UTC · re: Matthieu Moy · lore

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

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 17 quoted lines
> 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.
Show 86 quoted lines
>  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);
Matthieu Moy· Jul 25, 2014, 19:11 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to 3 files +30 −4

builtin/config.c, cache.h, ident.c

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
Matthieu Moy· Jul 25, 2014, 19:11 UTC · re: Matthieu Moy · lore

[PATCH v3 2/3] 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 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(-)
Show changes to path.c +6 −4
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
Matthieu Moy· Jul 25, 2014, 19:11 UTC · re: Matthieu Moy · lore

[PATCH v3 3/3] commit: advertise config --global --edit on guessed identity

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(-)
Show changes to builtin/commit.c +32 −2
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
Junio C Hamano· Jul 25, 2014, 19:38 UTC · re: Matthieu Moy · lore

Re: [PATCH v3 3/3] commit: advertise config --global --edit on guessed identity

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.

← back to recent threads