From: Felipe Contreras Date: Tue, 03 Feb 2009 23:43:06 GMT Subject: Re: [PATCH] config: Add new option to open an editor. Message-ID: <94a0d4530902031543t2928c9c4ta75132ddb4a9c52@mail.gmail.com> In-Reply-To: On Wed, Feb 4, 2009 at 1:31 AM, Johannes Schindelin wrote: > Hi, > > On Wed, 4 Feb 2009, Felipe Contreras wrote: > >> On Wed, Feb 4, 2009 at 1:05 AM, Johannes Schindelin >> wrote: >> > >> > On Wed, 4 Feb 2009, Felipe Contreras wrote: >> > >> >> On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin >> >> 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