Re: [PATCH v3] config: Add new option to open an editor.

Felipe Contreras [off-list ref] writes:

quoted

quoted

I think it should, how expensive would it be to call
git_config(git_default_config, NULL) when it has been loaded already?

We would not need the complete git_default_config(), would we?

Nope, just core.editor. But I don't know how to load only that.

The most expensive part is to actually open and parse the files into
tokens, not strcmp/prefixcmp the parsed tokens and flipping internal bits
and storing value in const char * variables.

But you need to be careful about correctness issues.  I do not want
launch_editor() to run git_config().  The caller, other than the caller
you happen to be interested in this thread, may already have read the
config and overrode some of the bits with what was given from the command
line.  Calling git_config() overwrite the bits and will break these
callers.

I think the right thing to do is to call git_config() immediately before
you call launch_editor() in your patch.

On Sat, Feb 7, 2009 at 11:34 PM, Junio C Hamano [off-list ref] wrote:

Felipe Contreras [off-list ref] writes:

quoted

quoted

quoted

I think it should, how expensive would it be to call
git_config(git_default_config, NULL) when it has been loaded already?

We would not need the complete git_default_config(), would we?

Nope, just core.editor. But I don't know how to load only that.

The most expensive part is to actually open and parse the files into
tokens, not strcmp/prefixcmp the parsed tokens and flipping internal bits
and storing value in const char * variables.

But you need to be careful about correctness issues.  I do not want
launch_editor() to run git_config().  The caller, other than the caller
you happen to be interested in this thread, may already have read the
config and overrode some of the bits with what was given from the command
line.  Calling git_config() overwrite the bits and will break these
callers.

I think the right thing to do is to call git_config() immediately before
you call launch_editor() in your patch.

Right, also for some reason the caller might actually call a specific
editor and not the one configured, but that would require more changes
to editor.c

For now I guess this patch should be fine (re-sending).

-- 
Felipe Contreras

The idea was originated by discussion about usability of manually
editing the config file in 'special needs' systems such as Windows. Now
the user can forget a bit about where the config files actually are.

Signed-off-by: Felipe Contreras <redacted>
---
 Documentation/git-config.txt |    6 ++++++
 builtin-config.c             |   13 ++++++++++++-
 2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 19a8917..7d14007 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt

@@ -22,6 +22,7 @@ SYNOPSIS
 'git config' [<file-option>] [-z|--null] -l | --list
 'git config' [<file-option>] --get-color name [default]
 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]
+'git config' [<file-option>] -e | --edit
 
 DESCRIPTION
 -----------

@@ -157,6 +158,11 @@ See also <<FILES>>.
 	output.  The optional `default` parameter is used instead, if
 	there is no color configured for `name`.
 
+-e::
+--edit::
+	Opens an editor to modify the specified config file; either
+	'--system', '--global', or repository (default).
+
 [[FILES]]
 FILES
 -----

diff --git a/builtin-config.c b/builtin-config.c
index f710162..6937eaf 100644
--- a/builtin-config.c
+++ b/builtin-config.c

@@ -3,7 +3,7 @@
 #include "color.h"
 
 static const char git_config_set_usage[] =
-"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default] | --get-colorbool name [stdout-is-tty]";
+"git config [ --global | --system | [ -f | --file ] config-file ] [ --bool | --int | --bool-or-int ] [ -z | --null ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --remove-section name | --list | --get-color var [default] | --get-colorbool name [stdout-is-tty] | --edit | -e ]";
 
 static char *key;
 static regex_t *key_regexp;

@@ -362,6 +362,17 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			return get_color(argc-2, argv+2);
 		} else if (!strcmp(argv[1], "--get-colorbool")) {
 			return get_colorbool(argc-2, argv+2);
+		} else if (!strcmp(argv[1], "--edit") || !strcmp(argv[1], "-e")) {
+			const char *config_filename;
+			if (argc != 2)
+				usage(git_config_set_usage);
+			if (config_exclusive_filename)
+				config_filename = config_exclusive_filename;
+			else
+				config_filename = git_path("config");
+			git_config(git_default_config, NULL);
+			launch_editor(config_filename, NULL, NULL);
+			return 0;
 		} else
 			break;
 		argc--;

-- 
1.6.1.2