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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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

> @@ -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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux