Re: [PATCH] config: Add new option to open an editor.
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Feb 3, 2009, 22:53 UTC
- Message-ID
- <alpine.DEB.1.00.0902032350130.9822@pacific.mpi-cbg.de>
- In-Reply-To
- <1233700826-11763-1-git-send-email-felipe.contreras@gmail.com>
Hi,
On Wed, 4 Feb 2009, Felipe Contreras wrote:
> 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.
Cute...
Show 10 quoted lines
> diff --git a/builtin-config.c b/builtin-config.c > index f710162..b0a86b1 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 ]";
This line is getting way too long... (Not nit-picking your current patch, but maybe you could provide another patch to break the line, while you're at builtin-config.c already. Maybe even parseopt'ifying it... ;-)
Show 13 quoted lines
> @@ -362,6 +362,15 @@ 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")) {
> + char *config_filename;
> + if (config_exclusive_filename)
> + config_filename = xstrdup(config_exclusive_filename);
> + else
> + config_filename = git_pathdup("config");
> + launch_editor(config_filename, NULL, NULL);
> + free(config_filename);
> + return 0;Does launch_editor() not take a 'const char *' on purpose? IOW you do not need to xstrdup() the filename. You do not even need git_pathdup(), as launch_editor() does not use git_path() itself.
However, a test case would be nice...
Ciao, Dscho