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

[Felipe Contreras <felipe.contreras@gmail.com>]

On Wed, Feb 4, 2009 at 1:31 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 4 Feb 2009, Felipe Contreras wrote:
>
>> On Wed, Feb 4, 2009 at 1:05 AM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> >
>> > On Wed, 4 Feb 2009, Felipe Contreras wrote:
>> >
>> >> On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin
>> >> <Johannes.Schindelin@gmx.de> wrote:
>> >>
>> >> > 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.
>>
>> Hmm, I'm not sure what issues this test case would find. If there's a
>> problem with launch_editor that's something other test case should find.
>
> The purpose of the test case is not to find problems now, but ensure that
> what the patch is intended to do does not get broken by subsequent
> patches.

I know, I wonder what kinds of possible problems this test case would find.

>> If there's no original file it's up to the editor to create one, if for
>> some reason the editor fails at doing that it's a problem of the editor,
>> and there's not much 'git config -e' could do except show an error, and
>> that's what launch_editor would do. Same thing if the editor is wrong
>> (GIT_EDITOR=blah).
>
> I was more thinking about 'git config --global -e' complaining that it
> could not find a non-existant file _before_ launching the editor.
>
> Likewise, GIT_EDITOR=echo was meant to output the filename, not to edit
> the file.

You mean if somebody for some reason decides to do some extra checking
on the config_filename before executing launch_editor? I seriously
doubt anything more would be needed for this --edit option. Can this
patch be acked as it is?

-- 
Felipe Contreras