Hi,
On Wed, 4 Feb 2009, Felipe Contreras wrote:
> On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
>
> > On Wed, 4 Feb 2009, Felipe Contreras wrote:
> >
> >> 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... ;-)
>
> Yeah, I'll send another patch to clean that string. Is there any
> example of that parseopt thing?
Yep... test-parse-options.c ;-)
> >> @@ -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.
>
> So, s/git_pathdup/git_path/ ?
And s/xstrdup(\(.*\))/\1/.
> > However, a test case would be nice...
>
> What would the the test case check?
That 'GIT_CONFIG=bla GIT_EDITOR=echo git config -e' and 'GIT_DIR=blub
GIT_EDITOR=echo git config -e' do the right thing. Maybe even --global,
but that would also be a good test that "git config --global -e" does not
fail when there is no original file.
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
