{"thread":{"id":"17539","subject":"[PATCH] config: Add new option to open an editor.","startedAt":"2009-02-03T22:40:26Z","lastAt":"2009-02-07T21:53:00Z","messageCount":25,"participants":["Felipe Contreras","Johannes Schindelin","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103029","messageId":"1233700826-11763-1-git-send-email-felipe.contreras@gmail.com","threadId":"17539","inReplyTo":null,"subject":"[PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-03T22:40:26Z","receivedAt":"2009-02-03T22:40:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The idea was originated by discussion about usability of manually\nediting the config file in 'special needs' systems such as Windows. Now\nthe user can forget a bit about where the config files actually are.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-config.txt |    6 ++++++\n builtin-config.c             |   11 ++++++++++-\n 2 files changed, 16 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 19a8917..7d14007 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -22,6 +22,7 @@ SYNOPSIS\n 'git config' [<file-option>] [-z|--null] -l | --list\n 'git config' [<file-option>] --get-color name [default]\n 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]\n+'git config' [<file-option>] -e | --edit\n \n DESCRIPTION\n -----------\n@@ -157,6 +158,11 @@ See also <<FILES>>.\n \toutput.  The optional `default` parameter is used instead, if\n \tthere is no color configured for `name`.\n \n+-e::\n+--edit::\n+\tOpens an editor to modify the specified config file; either\n+\t'--system', '--global', or repository (default).\n+\n [[FILES]]\n FILES\n -----\ndiff --git a/builtin-config.c b/builtin-config.c\nindex f710162..b0a86b1 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -3,7 +3,7 @@\n #include \"color.h\"\n \n static const char git_config_set_usage[] =\n-\"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]\";\n+\"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 ]\";\n \n static char *key;\n static regex_t *key_regexp;\n@@ -362,6 +362,15 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\treturn get_color(argc-2, argv+2);\n \t\t} else if (!strcmp(argv[1], \"--get-colorbool\")) {\n \t\t\treturn get_colorbool(argc-2, argv+2);\n+\t\t} else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n+\t\t\tchar *config_filename;\n+\t\t\tif (config_exclusive_filename)\n+\t\t\t\tconfig_filename = xstrdup(config_exclusive_filename);\n+\t\t\telse\n+\t\t\t\tconfig_filename = git_pathdup(\"config\");\n+\t\t\tlaunch_editor(config_filename, NULL, NULL);\n+\t\t\tfree(config_filename);\n+\t\t\treturn 0;\n \t\t} else\n \t\t\tbreak;\n \t\targc--;\n-- \n1.6.1.2\n"},{"id":"103031","messageId":"alpine.DEB.1.00.0902032350130.9822@pacific.mpi-cbg.de","threadId":"17539","inReplyTo":"1233700826-11763-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-03T22:53:26Z","receivedAt":"2009-02-03T22:53:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Feb 2009, Felipe Contreras wrote:\n\n> The idea was originated by discussion about usability of manually\n> editing the config file in 'special needs' systems such as Windows. Now\n> the user can forget a bit about where the config files actually are.\n\nCute...\n\n> diff --git a/builtin-config.c b/builtin-config.c\n> index f710162..b0a86b1 100644\n> --- a/builtin-config.c\n> +++ b/builtin-config.c\n> @@ -3,7 +3,7 @@\n>  #include \"color.h\"\n>  \n>  static const char git_config_set_usage[] =\n> -\"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]\";\n> +\"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 ]\";\n\nThis line is getting way too long...  (Not nit-picking your current patch, \nbut maybe you could provide another patch to break the line, while you're \nat builtin-config.c already.  Maybe even parseopt'ifying it... ;-)\n\n> @@ -362,6 +362,15 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n>  \t\t\treturn get_color(argc-2, argv+2);\n>  \t\t} else if (!strcmp(argv[1], \"--get-colorbool\")) {\n>  \t\t\treturn get_colorbool(argc-2, argv+2);\n> +\t\t} else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n> +\t\t\tchar *config_filename;\n> +\t\t\tif (config_exclusive_filename)\n> +\t\t\t\tconfig_filename = xstrdup(config_exclusive_filename);\n> +\t\t\telse\n> +\t\t\t\tconfig_filename = git_pathdup(\"config\");\n> +\t\t\tlaunch_editor(config_filename, NULL, NULL);\n> +\t\t\tfree(config_filename);\n> +\t\t\treturn 0;\n\nDoes launch_editor() not take a 'const char *' on purpose?  IOW you do not \nneed to xstrdup() the filename.  You do not even need git_pathdup(), as \nlaunch_editor() does not use git_path() itself.\n\nHowever, a test case would be nice...\n\nCiao,\nDscho\n"},{"id":"103033","messageId":"94a0d4530902031456u1e1753e2rc34786ebdce6e774@mail.gmail.com","threadId":"17539","inReplyTo":"alpine.DEB.1.00.0902032350130.9822@pacific.mpi-cbg.de","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-03T22:56:04Z","receivedAt":"2009-02-03T22:56:04Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Wed, 4 Feb 2009, Felipe Contreras wrote:\n>\n>> The idea was originated by discussion about usability of manually\n>> editing the config file in 'special needs' systems such as Windows. Now\n>> the user can forget a bit about where the config files actually are.\n>\n> Cute...\n>\n>> diff --git a/builtin-config.c b/builtin-config.c\n>> index f710162..b0a86b1 100644\n>> --- a/builtin-config.c\n>> +++ b/builtin-config.c\n>> @@ -3,7 +3,7 @@\n>>  #include \"color.h\"\n>>\n>>  static const char git_config_set_usage[] =\n>> -\"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]\";\n>> +\"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 ]\";\n>\n> This line is getting way too long...  (Not nit-picking your current patch,\n> but maybe you could provide another patch to break the line, while you're\n> at builtin-config.c already.  Maybe even parseopt'ifying it... ;-)\n\nYeah, I'll send another patch to clean that string. Is there any\nexample of that parseopt thing?\n\n>> @@ -362,6 +362,15 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n>>                       return get_color(argc-2, argv+2);\n>>               } else if (!strcmp(argv[1], \"--get-colorbool\")) {\n>>                       return get_colorbool(argc-2, argv+2);\n>> +             } else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n>> +                     char *config_filename;\n>> +                     if (config_exclusive_filename)\n>> +                             config_filename = xstrdup(config_exclusive_filename);\n>> +                     else\n>> +                             config_filename = git_pathdup(\"config\");\n>> +                     launch_editor(config_filename, NULL, NULL);\n>> +                     free(config_filename);\n>> +                     return 0;\n>\n> Does launch_editor() not take a 'const char *' on purpose?  IOW you do not\n> need to xstrdup() the filename.  You do not even need git_pathdup(), as\n> launch_editor() does not use git_path() itself.\n\nSo, s/git_pathdup/git_path/ ?\n\n> However, a test case would be nice...\n\nWhat would the the test case check?\n\n-- \nFelipe Contreras\n"},{"id":"103035","messageId":"alpine.DEB.1.00.0902040002360.9822@pacific.mpi-cbg.de","threadId":"17539","inReplyTo":"94a0d4530902031456u1e1753e2rc34786ebdce6e774@mail.gmail.com","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-03T23:05:51Z","receivedAt":"2009-02-03T23:05:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Feb 2009, Felipe Contreras wrote:\n\n> On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n>\n> > On Wed, 4 Feb 2009, Felipe Contreras wrote:\n> >\n> >>  static const char git_config_set_usage[] =\n> >> -\"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]\";\n> >> +\"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 ]\";\n> >\n> > This line is getting way too long...  (Not nit-picking your current patch,\n> > but maybe you could provide another patch to break the line, while you're\n> > at builtin-config.c already.  Maybe even parseopt'ifying it... ;-)\n> \n> Yeah, I'll send another patch to clean that string. Is there any\n> example of that parseopt thing?\n\nYep... test-parse-options.c ;-)\n\n> >> @@ -362,6 +362,15 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> >>                       return get_color(argc-2, argv+2);\n> >>               } else if (!strcmp(argv[1], \"--get-colorbool\")) {\n> >>                       return get_colorbool(argc-2, argv+2);\n> >> +             } else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n> >> +                     char *config_filename;\n> >> +                     if (config_exclusive_filename)\n> >> +                             config_filename = xstrdup(config_exclusive_filename);\n> >> +                     else\n> >> +                             config_filename = git_pathdup(\"config\");\n> >> +                     launch_editor(config_filename, NULL, NULL);\n> >> +                     free(config_filename);\n> >> +                     return 0;\n> >\n> > Does launch_editor() not take a 'const char *' on purpose?  IOW you do not\n> > need to xstrdup() the filename.  You do not even need git_pathdup(), as\n> > launch_editor() does not use git_path() itself.\n> \n> So, s/git_pathdup/git_path/ ?\n\nAnd s/xstrdup(\\(.*\\))/\\1/.\n\n> > However, a test case would be nice...\n> \n> What would the the test case check?\n\nThat 'GIT_CONFIG=bla GIT_EDITOR=echo git config -e' and 'GIT_DIR=blub \nGIT_EDITOR=echo git config -e' do the right thing.  Maybe even --global, \nbut that would also be a good test that \"git config --global -e\" does not \nfail when there is no original file.\n\nCiao,\nDscho\n"},{"id":"103041","messageId":"94a0d4530902031525o11e512barfc2f50c33a520474@mail.gmail.com","threadId":"17539","inReplyTo":"alpine.DEB.1.00.0902040002360.9822@pacific.mpi-cbg.de","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-03T23:25:26Z","receivedAt":"2009-02-03T23:25:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Feb 4, 2009 at 1:05 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Wed, 4 Feb 2009, Felipe Contreras wrote:\n>\n>> On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin\n>> <Johannes.Schindelin@gmx.de> wrote:\n>>\n>> > On Wed, 4 Feb 2009, Felipe Contreras wrote:\n>> >\n>> >>  static const char git_config_set_usage[] =\n>> >> -\"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]\";\n>> >> +\"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 ]\";\n>> >\n>> > This line is getting way too long...  (Not nit-picking your current patch,\n>> > but maybe you could provide another patch to break the line, while you're\n>> > at builtin-config.c already.  Maybe even parseopt'ifying it... ;-)\n>>\n>> Yeah, I'll send another patch to clean that string. Is there any\n>> example of that parseopt thing?\n>\n> Yep... test-parse-options.c ;-)\n\nCool, I'll check that.\n\n>> >> @@ -362,6 +362,15 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n>> >>                       return get_color(argc-2, argv+2);\n>> >>               } else if (!strcmp(argv[1], \"--get-colorbool\")) {\n>> >>                       return get_colorbool(argc-2, argv+2);\n>> >> +             } else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n>> >> +                     char *config_filename;\n>> >> +                     if (config_exclusive_filename)\n>> >> +                             config_filename = xstrdup(config_exclusive_filename);\n>> >> +                     else\n>> >> +                             config_filename = git_pathdup(\"config\");\n>> >> +                     launch_editor(config_filename, NULL, NULL);\n>> >> +                     free(config_filename);\n>> >> +                     return 0;\n>> >\n>> > Does launch_editor() not take a 'const char *' on purpose?  IOW you do not\n>> > need to xstrdup() the filename.  You do not even need git_pathdup(), as\n>> > launch_editor() does not use git_path() itself.\n>>\n>> So, s/git_pathdup/git_path/ ?\n>\n> And s/xstrdup(\\(.*\\))/\\1/.\n\nYeah, I understood that.\n\n>> > However, a test case would be nice...\n>>\n>> What would the the test case check?\n>\n> That 'GIT_CONFIG=bla GIT_EDITOR=echo git config -e' and 'GIT_DIR=blub\n> GIT_EDITOR=echo git config -e' do the right thing.  Maybe even --global,\n> but that would also be a good test that \"git config --global -e\" does not\n> fail when there is no original file.\n\nHmm, I'm not sure what issues this test case would find. If there's a\nproblem with launch_editor that's something other test case should\nfind.\n\nIf there's no original file it's up to the editor to create one, if\nfor some reason the editor fails at doing that it's a problem of the\neditor, and there's not much 'git config -e' could do except show an\nerror, and that's what launch_editor would do. Same thing if the\neditor is wrong (GIT_EDITOR=blah).\n\nI'll resend.\n\n-- \nFelipe Contreras\n"},{"id":"103049","messageId":"1233703597-14532-1-git-send-email-felipe.contreras@gmail.com","threadId":"17539","inReplyTo":"94a0d4530902031525o11e512barfc2f50c33a520474@mail.gmail.com","subject":"[PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-03T23:26:37Z","receivedAt":"2009-02-03T23:26:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The idea was originated by discussion about usability of manually\nediting the config file in 'special needs' systems such as Windows. Now\nthe user can forget a bit about where the config files actually are.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-config.txt |    6 ++++++\n builtin-config.c             |   10 +++++++++-\n 2 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 19a8917..7d14007 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -22,6 +22,7 @@ SYNOPSIS\n 'git config' [<file-option>] [-z|--null] -l | --list\n 'git config' [<file-option>] --get-color name [default]\n 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]\n+'git config' [<file-option>] -e | --edit\n \n DESCRIPTION\n -----------\n@@ -157,6 +158,11 @@ See also <<FILES>>.\n \toutput.  The optional `default` parameter is used instead, if\n \tthere is no color configured for `name`.\n \n+-e::\n+--edit::\n+\tOpens an editor to modify the specified config file; either\n+\t'--system', '--global', or repository (default).\n+\n [[FILES]]\n FILES\n -----\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 1582673..0405242 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -3,7 +3,7 @@\n #include \"color.h\"\n \n static const char git_config_set_usage[] =\n-\"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]\";\n+\"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 ]\";\n \n static char *key;\n static regex_t *key_regexp;\n@@ -362,6 +362,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\treturn get_color(argc-2, argv+2);\n \t\t} else if (!strcmp(argv[1], \"--get-colorbool\")) {\n \t\t\treturn get_colorbool(argc-2, argv+2);\n+\t\t} else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n+\t\t\tconst char *config_filename;\n+\t\t\tif (config_exclusive_filename)\n+\t\t\t\tconfig_filename = config_exclusive_filename;\n+\t\t\telse\n+\t\t\t\tconfig_filename = git_path(\"config\");\n+\t\t\tlaunch_editor(config_filename, NULL, NULL);\n+\t\t\treturn 0;\n \t\t} else\n \t\t\tbreak;\n \t\targc--;\n-- \n1.6.1.2\n"},{"id":"103050","messageId":"alpine.DEB.1.00.0902040029040.9822@pacific.mpi-cbg.de","threadId":"17539","inReplyTo":"94a0d4530902031525o11e512barfc2f50c33a520474@mail.gmail.com","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-03T23:31:15Z","receivedAt":"2009-02-03T23:31:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Feb 2009, Felipe Contreras wrote:\n\n> On Wed, Feb 4, 2009 at 1:05 AM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> > On Wed, 4 Feb 2009, Felipe Contreras wrote:\n> >\n> >> On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin\n> >> <Johannes.Schindelin@gmx.de> wrote:\n> >>\n> >> > However, a test case would be nice...\n> >>\n> >> What would the the test case check?\n> >\n> > That 'GIT_CONFIG=bla GIT_EDITOR=echo git config -e' and 'GIT_DIR=blub \n> > GIT_EDITOR=echo git config -e' do the right thing.  Maybe even \n> > --global, but that would also be a good test that \"git config --global \n> > -e\" does not fail when there is no original file.\n> \n> Hmm, I'm not sure what issues this test case would find. If there's a \n> problem with launch_editor that's something other test case should find.\n\nThe purpose of the test case is not to find problems now, but ensure that \nwhat the patch is intended to do does not get broken by subsequent \npatches.\n\n> If there's no original file it's up to the editor to create one, if for \n> some reason the editor fails at doing that it's a problem of the editor, \n> and there's not much 'git config -e' could do except show an error, and \n> that's what launch_editor would do. Same thing if the editor is wrong \n> (GIT_EDITOR=blah).\n\nI was more thinking about 'git config --global -e' complaining that it \ncould not find a non-existant file _before_ launching the editor.\n\nLikewise, GIT_EDITOR=echo was meant to output the filename, not to edit \nthe file.\n\nCiao,\nDscho\n"},{"id":"103051","messageId":"94a0d4530902031543t2928c9c4ta75132ddb4a9c52@mail.gmail.com","threadId":"17539","inReplyTo":"alpine.DEB.1.00.0902040029040.9822@pacific.mpi-cbg.de","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-03T23:43:06Z","receivedAt":"2009-02-03T23:43:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Feb 4, 2009 at 1:31 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Wed, 4 Feb 2009, Felipe Contreras wrote:\n>\n>> On Wed, Feb 4, 2009 at 1:05 AM, Johannes Schindelin\n>> <Johannes.Schindelin@gmx.de> wrote:\n>> >\n>> > On Wed, 4 Feb 2009, Felipe Contreras wrote:\n>> >\n>> >> On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin\n>> >> <Johannes.Schindelin@gmx.de> wrote:\n>> >>\n>> >> > However, a test case would be nice...\n>> >>\n>> >> What would the the test case check?\n>> >\n>> > That 'GIT_CONFIG=bla GIT_EDITOR=echo git config -e' and 'GIT_DIR=blub\n>> > GIT_EDITOR=echo git config -e' do the right thing.  Maybe even\n>> > --global, but that would also be a good test that \"git config --global\n>> > -e\" does not fail when there is no original file.\n>>\n>> Hmm, I'm not sure what issues this test case would find. If there's a\n>> problem with launch_editor that's something other test case should find.\n>\n> The purpose of the test case is not to find problems now, but ensure that\n> what the patch is intended to do does not get broken by subsequent\n> patches.\n\nI know, I wonder what kinds of possible problems this test case would find.\n\n>> If there's no original file it's up to the editor to create one, if for\n>> some reason the editor fails at doing that it's a problem of the editor,\n>> and there's not much 'git config -e' could do except show an error, and\n>> that's what launch_editor would do. Same thing if the editor is wrong\n>> (GIT_EDITOR=blah).\n>\n> I was more thinking about 'git config --global -e' complaining that it\n> could not find a non-existant file _before_ launching the editor.\n>\n> Likewise, GIT_EDITOR=echo was meant to output the filename, not to edit\n> the file.\n\nYou mean if somebody for some reason decides to do some extra checking\non the config_filename before executing launch_editor? I seriously\ndoubt anything more would be needed for this --edit option. Can this\npatch be acked as it is?\n\n-- \nFelipe Contreras\n"},{"id":"103143","messageId":"20090204145317.GA5712@sigill.intra.peff.net","threadId":"17539","inReplyTo":"1233700826-11763-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-04T14:53:17Z","receivedAt":"2009-02-04T14:53:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 04, 2009 at 12:40:26AM +0200, Felipe Contreras wrote:\n\n> +\t\t} else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n> +\t\t\tchar *config_filename;\n> +\t\t\tif (config_exclusive_filename)\n> +\t\t\t\tconfig_filename = xstrdup(config_exclusive_filename);\n> +\t\t\telse\n> +\t\t\t\tconfig_filename = git_pathdup(\"config\");\n> +\t\t\tlaunch_editor(config_filename, NULL, NULL);\n> +\t\t\tfree(config_filename);\n> +\t\t\treturn 0;\n>  \t\t} else\n\nWith this patch, won't I get different behavior from:\n\n  git config -e --global\n\nversus\n\n  git config --global -e\n\n?\n\nOther than that, I like the concept of this patch.\n\n-Peff\n"},{"id":"103145","messageId":"94a0d4530902040703u53a9082fsfa7203b6203bd118@mail.gmail.com","threadId":"17539","inReplyTo":"20090204145317.GA5712@sigill.intra.peff.net","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-04T15:03:22Z","receivedAt":"2009-02-04T15:03:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Feb 4, 2009 at 4:53 PM, Jeff King <peff@peff.net> wrote:\n> On Wed, Feb 04, 2009 at 12:40:26AM +0200, Felipe Contreras wrote:\n>\n>> +             } else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n>> +                     char *config_filename;\n>> +                     if (config_exclusive_filename)\n>> +                             config_filename = xstrdup(config_exclusive_filename);\n>> +                     else\n>> +                             config_filename = git_pathdup(\"config\");\n>> +                     launch_editor(config_filename, NULL, NULL);\n>> +                     free(config_filename);\n>> +                     return 0;\n>>               } else\n>\n> With this patch, won't I get different behavior from:\n>\n>  git config -e --global\n>\n> versus\n>\n>  git config --global -e\n\nJust like you get different behavior from:\n\ngit config -l --global\n\nand\n\ngit config --global -l\n\n-- \nFelipe Contreras\n"},{"id":"103146","messageId":"20090204151833.GA6896@sigill.intra.peff.net","threadId":"17539","inReplyTo":"94a0d4530902040703u53a9082fsfa7203b6203bd118@mail.gmail.com","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-04T15:18:33Z","receivedAt":"2009-02-04T15:18:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 04, 2009 at 05:03:22PM +0200, Felipe Contreras wrote:\n\n> > With this patch, won't I get different behavior from:\n> >\n> >  git config -e --global\n> >\n> > versus\n> >\n> >  git config --global -e\n> \n> Just like you get different behavior from:\n> \n> git config -l --global\n> \n> and\n> \n> git config --global -l\n\nUgh. Personally I consider such interfaces poorly designed. I understand\nthat the general way \"git config\" works is to have \"git config [options]\n[action]\". And when \"[action]\" is a variable name, or a variable name\nwith a value, it is easy to see what's going on. But when the action\nlooks like an option, it is just confusing that their ordering is\nimportant.\n\nHowever, the interface to \"git config\" is not going to change, so I\nthink your following existing practice is reasonable here.\n\n_But_ there is one important difference between your \"-e\" and \"-l\". In\nthe \"-l\" case, we detect that there is extra trailing cruft that will be\nignored and give a usage message.  So \"git config -l --global\"\ncomplains, but \"git config -e --global\" silently ignores the second\nargument. I think you just need to add\n\n  if (argc != 2)\n    usage(git_config_set_usage);\n\nas the \"-l\" code does.\n\n-Peff\n"},{"id":"103147","messageId":"alpine.DEB.1.00.0902041619370.22763@intel-tinevez-2-302","threadId":"17539","inReplyTo":"94a0d4530902040703u53a9082fsfa7203b6203bd118@mail.gmail.com","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-04T15:20:07Z","receivedAt":"2009-02-04T15:20:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Feb 2009, Felipe Contreras wrote:\n\n> On Wed, Feb 4, 2009 at 4:53 PM, Jeff King <peff@peff.net> wrote:\n> > On Wed, Feb 04, 2009 at 12:40:26AM +0200, Felipe Contreras wrote:\n> >\n> >> +             } else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n> >> +                     char *config_filename;\n> >> +                     if (config_exclusive_filename)\n> >> +                             config_filename = xstrdup(config_exclusive_filename);\n> >> +                     else\n> >> +                             config_filename = git_pathdup(\"config\");\n> >> +                     launch_editor(config_filename, NULL, NULL);\n> >> +                     free(config_filename);\n> >> +                     return 0;\n> >>               } else\n> >\n> > With this patch, won't I get different behavior from:\n> >\n> >  git config -e --global\n> >\n> > versus\n> >\n> >  git config --global -e\n> \n> Just like you get different behavior from:\n> \n> git config -l --global\n> \n> and\n> \n> git config --global -l\n\n... which will be fixed once you parseopt'ified builtin-config, right?\n\n:-)\n\nCiao,\nDscho\n"},{"id":"103153","messageId":"94a0d4530902040741t4333b132v886d7057353ed52a@mail.gmail.com","threadId":"17539","inReplyTo":"20090204151833.GA6896@sigill.intra.peff.net","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-04T15:41:55Z","receivedAt":"2009-02-04T15:41:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Feb 4, 2009 at 5:18 PM, Jeff King <peff@peff.net> wrote:\n> On Wed, Feb 04, 2009 at 05:03:22PM +0200, Felipe Contreras wrote:\n>\n>> > With this patch, won't I get different behavior from:\n>> >\n>> >  git config -e --global\n>> >\n>> > versus\n>> >\n>> >  git config --global -e\n>>\n>> Just like you get different behavior from:\n>>\n>> git config -l --global\n>>\n>> and\n>>\n>> git config --global -l\n>\n> Ugh. Personally I consider such interfaces poorly designed. I understand\n> that the general way \"git config\" works is to have \"git config [options]\n> [action]\". And when \"[action]\" is a variable name, or a variable name\n> with a value, it is easy to see what's going on. But when the action\n> looks like an option, it is just confusing that their ordering is\n> important.\n>\n> However, the interface to \"git config\" is not going to change, so I\n> think your following existing practice is reasonable here.\n>\n> _But_ there is one important difference between your \"-e\" and \"-l\". In\n> the \"-l\" case, we detect that there is extra trailing cruft that will be\n> ignored and give a usage message.  So \"git config -l --global\"\n> complains, but \"git config -e --global\" silently ignores the second\n> argument. I think you just need to add\n>\n>  if (argc != 2)\n>    usage(git_config_set_usage);\n>\n> as the \"-l\" code does.\n\nOh, ok, will do.\n\n-- \nFelipe Contreras\n"},{"id":"103155","messageId":"94a0d4530902040742o4f2f0b63g44d605e5b28d9b15@mail.gmail.com","threadId":"17539","inReplyTo":"alpine.DEB.1.00.0902041619370.22763@intel-tinevez-2-302","subject":"Re: [PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-04T15:42:59Z","receivedAt":"2009-02-04T15:42:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Feb 4, 2009 at 5:20 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Wed, 4 Feb 2009, Felipe Contreras wrote:\n>\n>> On Wed, Feb 4, 2009 at 4:53 PM, Jeff King <peff@peff.net> wrote:\n>> > On Wed, Feb 04, 2009 at 12:40:26AM +0200, Felipe Contreras wrote:\n>> >\n>> >> +             } else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n>> >> +                     char *config_filename;\n>> >> +                     if (config_exclusive_filename)\n>> >> +                             config_filename = xstrdup(config_exclusive_filename);\n>> >> +                     else\n>> >> +                             config_filename = git_pathdup(\"config\");\n>> >> +                     launch_editor(config_filename, NULL, NULL);\n>> >> +                     free(config_filename);\n>> >> +                     return 0;\n>> >>               } else\n>> >\n>> > With this patch, won't I get different behavior from:\n>> >\n>> >  git config -e --global\n>> >\n>> > versus\n>> >\n>> >  git config --global -e\n>>\n>> Just like you get different behavior from:\n>>\n>> git config -l --global\n>>\n>> and\n>>\n>> git config --global -l\n>\n> ... which will be fixed once you parseopt'ified builtin-config, right?\n\nI'm going to cleanup the big string, I still haven't looked at\nparseopt, if it's not too complicated I'll do it.\n\n-- \nFelipe Contreras\n"},{"id":"103217","messageId":"1233786856-6571-1-git-send-email-felipe.contreras@gmail.com","threadId":"17539","inReplyTo":"94a0d4530902040741t4333b132v886d7057353ed52a@mail.gmail.com","subject":"[PATCH v3] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-04T22:34:16Z","receivedAt":"2009-02-04T22:34:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The idea was originated by discussion about usability of manually\nediting the config file in 'special needs' systems such as Windows. Now\nthe user can forget a bit about where the config files actually are.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-config.txt |    6 ++++++\n builtin-config.c             |   12 +++++++++++-\n 2 files changed, 17 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 19a8917..7d14007 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -22,6 +22,7 @@ SYNOPSIS\n 'git config' [<file-option>] [-z|--null] -l | --list\n 'git config' [<file-option>] --get-color name [default]\n 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]\n+'git config' [<file-option>] -e | --edit\n \n DESCRIPTION\n -----------\n@@ -157,6 +158,11 @@ See also <<FILES>>.\n \toutput.  The optional `default` parameter is used instead, if\n \tthere is no color configured for `name`.\n \n+-e::\n+--edit::\n+\tOpens an editor to modify the specified config file; either\n+\t'--system', '--global', or repository (default).\n+\n [[FILES]]\n FILES\n -----\ndiff --git a/builtin-config.c b/builtin-config.c\nindex 1582673..4457b34 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -3,7 +3,7 @@\n #include \"color.h\"\n \n static const char git_config_set_usage[] =\n-\"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]\";\n+\"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 ]\";\n \n static char *key;\n static regex_t *key_regexp;\n@@ -362,6 +362,16 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\treturn get_color(argc-2, argv+2);\n \t\t} else if (!strcmp(argv[1], \"--get-colorbool\")) {\n \t\t\treturn get_colorbool(argc-2, argv+2);\n+\t\t} else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n+\t\t\tconst char *config_filename;\n+\t\t\tif (argc != 2)\n+\t\t\t\tusage(git_config_set_usage);\n+\t\t\tif (config_exclusive_filename)\n+\t\t\t\tconfig_filename = config_exclusive_filename;\n+\t\t\telse\n+\t\t\t\tconfig_filename = git_path(\"config\");\n+\t\t\tlaunch_editor(config_filename, NULL, NULL);\n+\t\t\treturn 0;\n \t\t} else\n \t\t\tbreak;\n \t\targc--;\n-- \n1.6.1.2\n"},{"id":"103231","messageId":"7vmyd1lrnx.fsf@gitster.siamese.dyndns.org","threadId":"17539","inReplyTo":"1233786856-6571-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-04T23:16:02Z","receivedAt":"2009-02-04T23:16:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> The idea was originated by discussion about usability of manually\n> editing the config file in 'special needs' systems such as Windows. Now\n> the user can forget a bit about where the config files actually are.\n\nDoes this honor core.editor setting in existing configuration files?\n"},{"id":"103238","messageId":"94a0d4530902041539w6e4e747fg804faaaf272e3f58@mail.gmail.com","threadId":"17539","inReplyTo":"7vmyd1lrnx.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-04T23:39:29Z","receivedAt":"2009-02-04T23:39:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Feb 5, 2009 at 1:16 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> The idea was originated by discussion about usability of manually\n>> editing the config file in 'special needs' systems such as Windows. Now\n>> the user can forget a bit about where the config files actually are.\n>\n> Does this honor core.editor setting in existing configuration files?\n\nNope. I guess I need to add git_config(git_default_config, NULL) for that.\n\n-- \nFelipe Contreras\n"},{"id":"103239","messageId":"alpine.DEB.1.00.0902050043000.10279@pacific.mpi-cbg.de","threadId":"17539","inReplyTo":"7vmyd1lrnx.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-04T23:43:36Z","receivedAt":"2009-02-04T23:43:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Feb 2009, Junio C Hamano wrote:\n\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> \n> > The idea was originated by discussion about usability of manually\n> > editing the config file in 'special needs' systems such as Windows. Now\n> > the user can forget a bit about where the config files actually are.\n> \n> Does this honor core.editor setting in existing configuration files?\n\nIt should, as it uses launch_editor().\n\nCiao,\nDscho\n"},{"id":"103241","messageId":"alpine.DEB.1.00.0902050049240.10279@pacific.mpi-cbg.de","threadId":"17539","inReplyTo":"alpine.DEB.1.00.0902050043000.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-04T23:49:53Z","receivedAt":"2009-02-04T23:49:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 5 Feb 2009, Johannes Schindelin wrote:\n\n> On Wed, 4 Feb 2009, Junio C Hamano wrote:\n> \n> > Felipe Contreras <felipe.contreras@gmail.com> writes:\n> > \n> > > The idea was originated by discussion about usability of manually\n> > > editing the config file in 'special needs' systems such as Windows. Now\n> > > the user can forget a bit about where the config files actually are.\n> > \n> > Does this honor core.editor setting in existing configuration files?\n> \n> It should, as it uses launch_editor().\n\nOh, I see that launch_editor() does not do its own git_config() parsing.  \nDunno if it should...\n\nCiao,\nDscho\n"},{"id":"103651","messageId":"94a0d4530902071309l7c34945ana2af31e41707fa0c@mail.gmail.com","threadId":"17539","inReplyTo":"alpine.DEB.1.00.0902050049240.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-07T21:09:49Z","receivedAt":"2009-02-07T21:09:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Feb 5, 2009 at 1:49 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Thu, 5 Feb 2009, Johannes Schindelin wrote:\n>\n>> On Wed, 4 Feb 2009, Junio C Hamano wrote:\n>>\n>> > Felipe Contreras <felipe.contreras@gmail.com> writes:\n>> >\n>> > > The idea was originated by discussion about usability of manually\n>> > > editing the config file in 'special needs' systems such as Windows. Now\n>> > > the user can forget a bit about where the config files actually are.\n>> >\n>> > Does this honor core.editor setting in existing configuration files?\n>>\n>> It should, as it uses launch_editor().\n>\n> Oh, I see that launch_editor() does not do its own git_config() parsing.\n> Dunno if it should...\n\nI think it should, how expensive would it be to call\ngit_config(git_default_config, NULL) when it has been loaded already?\n\n-- \nFelipe Contreras\n"},{"id":"103649","messageId":"alpine.DEB.1.00.0902072214410.10279@pacific.mpi-cbg.de","threadId":"17539","inReplyTo":"94a0d4530902071309l7c34945ana2af31e41707fa0c@mail.gmail.com","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-07T21:14:58Z","receivedAt":"2009-02-07T21:14:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 7 Feb 2009, Felipe Contreras wrote:\n\n> On Thu, Feb 5, 2009 at 1:49 AM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > Hi,\n> >\n> > On Thu, 5 Feb 2009, Johannes Schindelin wrote:\n> >\n> >> On Wed, 4 Feb 2009, Junio C Hamano wrote:\n> >>\n> >> > Felipe Contreras <felipe.contreras@gmail.com> writes:\n> >> >\n> >> > > The idea was originated by discussion about usability of manually\n> >> > > editing the config file in 'special needs' systems such as Windows. Now\n> >> > > the user can forget a bit about where the config files actually are.\n> >> >\n> >> > Does this honor core.editor setting in existing configuration files?\n> >>\n> >> It should, as it uses launch_editor().\n> >\n> > Oh, I see that launch_editor() does not do its own git_config() parsing.\n> > Dunno if it should...\n> \n> I think it should, how expensive would it be to call\n> git_config(git_default_config, NULL) when it has been loaded already?\n\nWe would not need the complete git_default_config(), would we?\n\nCiao,\nDscho\n"},{"id":"103650","messageId":"94a0d4530902071315o78b2db58n481fbc6e956b5b64@mail.gmail.com","threadId":"17539","inReplyTo":"alpine.DEB.1.00.0902072214410.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-07T21:15:30Z","receivedAt":"2009-02-07T21:15:30Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Feb 7, 2009 at 11:14 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Sat, 7 Feb 2009, Felipe Contreras wrote:\n>\n>> On Thu, Feb 5, 2009 at 1:49 AM, Johannes Schindelin\n>> <Johannes.Schindelin@gmx.de> wrote:\n>> > Hi,\n>> >\n>> > On Thu, 5 Feb 2009, Johannes Schindelin wrote:\n>> >\n>> >> On Wed, 4 Feb 2009, Junio C Hamano wrote:\n>> >>\n>> >> > Felipe Contreras <felipe.contreras@gmail.com> writes:\n>> >> >\n>> >> > > The idea was originated by discussion about usability of manually\n>> >> > > editing the config file in 'special needs' systems such as Windows. Now\n>> >> > > the user can forget a bit about where the config files actually are.\n>> >> >\n>> >> > Does this honor core.editor setting in existing configuration files?\n>> >>\n>> >> It should, as it uses launch_editor().\n>> >\n>> > Oh, I see that launch_editor() does not do its own git_config() parsing.\n>> > Dunno if it should...\n>>\n>> I think it should, how expensive would it be to call\n>> git_config(git_default_config, NULL) when it has been loaded already?\n>\n> We would not need the complete git_default_config(), would we?\n\nNope, just core.editor. But I don't know how to load only that.\n\n-- \nFelipe Contreras\n"},{"id":"103652","messageId":"7vskmque25.fsf@gitster.siamese.dyndns.org","threadId":"17539","inReplyTo":"94a0d4530902071315o78b2db58n481fbc6e956b5b64@mail.gmail.com","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-07T21:34:10Z","receivedAt":"2009-02-07T21:34:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>>> I think it should, how expensive would it be to call\n>>> git_config(git_default_config, NULL) when it has been loaded already?\n>>\n>> We would not need the complete git_default_config(), would we?\n>\n> Nope, just core.editor. But I don't know how to load only that.\n\nThe most expensive part is to actually open and parse the files into\ntokens, not strcmp/prefixcmp the parsed tokens and flipping internal bits\nand storing value in const char * variables.\n\nBut you need to be careful about correctness issues.  I do not want\nlaunch_editor() to run git_config().  The caller, other than the caller\nyou happen to be interested in this thread, may already have read the\nconfig and overrode some of the bits with what was given from the command\nline.  Calling git_config() overwrite the bits and will break these\ncallers.\n\nI think the right thing to do is to call git_config() immediately before\nyou call launch_editor() in your patch.\n"},{"id":"103659","messageId":"94a0d4530902071350h4caab5a8s597ed099d232dde9@mail.gmail.com","threadId":"17539","inReplyTo":"7vskmque25.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-07T21:50:36Z","receivedAt":"2009-02-07T21:50:36Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Feb 7, 2009 at 11:34 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>>>> I think it should, how expensive would it be to call\n>>>> git_config(git_default_config, NULL) when it has been loaded already?\n>>>\n>>> We would not need the complete git_default_config(), would we?\n>>\n>> Nope, just core.editor. But I don't know how to load only that.\n>\n> The most expensive part is to actually open and parse the files into\n> tokens, not strcmp/prefixcmp the parsed tokens and flipping internal bits\n> and storing value in const char * variables.\n>\n> But you need to be careful about correctness issues.  I do not want\n> launch_editor() to run git_config().  The caller, other than the caller\n> you happen to be interested in this thread, may already have read the\n> config and overrode some of the bits with what was given from the command\n> line.  Calling git_config() overwrite the bits and will break these\n> callers.\n>\n> I think the right thing to do is to call git_config() immediately before\n> you call launch_editor() in your patch.\n\nRight, also for some reason the caller might actually call a specific\neditor and not the one configured, but that would require more changes\nto editor.c\n\nFor now I guess this patch should be fine (re-sending).\n\n-- \nFelipe Contreras\n"},{"id":"103660","messageId":"1234043580-7241-1-git-send-email-felipe.contreras@gmail.com","threadId":"17539","inReplyTo":"94a0d4530902071350h4caab5a8s597ed099d232dde9@mail.gmail.com","subject":"[PATCH] config: Add new option to open an editor.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-07T21:53:00Z","receivedAt":"2009-02-07T21:53:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The idea was originated by discussion about usability of manually\nediting the config file in 'special needs' systems such as Windows. Now\nthe user can forget a bit about where the config files actually are.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-config.txt |    6 ++++++\n builtin-config.c             |   13 ++++++++++++-\n 2 files changed, 18 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 19a8917..7d14007 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -22,6 +22,7 @@ SYNOPSIS\n 'git config' [<file-option>] [-z|--null] -l | --list\n 'git config' [<file-option>] --get-color name [default]\n 'git config' [<file-option>] --get-colorbool name [stdout-is-tty]\n+'git config' [<file-option>] -e | --edit\n \n DESCRIPTION\n -----------\n@@ -157,6 +158,11 @@ See also <<FILES>>.\n \toutput.  The optional `default` parameter is used instead, if\n \tthere is no color configured for `name`.\n \n+-e::\n+--edit::\n+\tOpens an editor to modify the specified config file; either\n+\t'--system', '--global', or repository (default).\n+\n [[FILES]]\n FILES\n -----\ndiff --git a/builtin-config.c b/builtin-config.c\nindex f710162..6937eaf 100644\n--- a/builtin-config.c\n+++ b/builtin-config.c\n@@ -3,7 +3,7 @@\n #include \"color.h\"\n \n static const char git_config_set_usage[] =\n-\"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]\";\n+\"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 ]\";\n \n static char *key;\n static regex_t *key_regexp;\n@@ -362,6 +362,17 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\t\treturn get_color(argc-2, argv+2);\n \t\t} else if (!strcmp(argv[1], \"--get-colorbool\")) {\n \t\t\treturn get_colorbool(argc-2, argv+2);\n+\t\t} else if (!strcmp(argv[1], \"--edit\") || !strcmp(argv[1], \"-e\")) {\n+\t\t\tconst char *config_filename;\n+\t\t\tif (argc != 2)\n+\t\t\t\tusage(git_config_set_usage);\n+\t\t\tif (config_exclusive_filename)\n+\t\t\t\tconfig_filename = config_exclusive_filename;\n+\t\t\telse\n+\t\t\t\tconfig_filename = git_path(\"config\");\n+\t\t\tgit_config(git_default_config, NULL);\n+\t\t\tlaunch_editor(config_filename, NULL, NULL);\n+\t\t\treturn 0;\n \t\t} else\n \t\t\tbreak;\n \t\targc--;\n-- \n1.6.1.2\n"}]}