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

On Wed, Feb 4, 2009 at 1:31 AM, Johannes Schindelin
[off-list ref] wrote:

Hi,

On Wed, 4 Feb 2009, Felipe Contreras wrote:

quoted

On Wed, Feb 4, 2009 at 1:05 AM, Johannes Schindelin
[off-list ref] wrote:

quoted

On Wed, 4 Feb 2009, Felipe Contreras wrote:

quoted

On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin
[off-list ref] wrote:

quoted

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.

quoted

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