On Wed, Feb 4, 2009 at 1:31 AM, Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > Hi, > > On Wed, 4 Feb 2009, Felipe Contreras wrote: > >> On Wed, Feb 4, 2009 at 1:05 AM, Johannes Schindelin >> <Johannes.Schindelin@xxxxxx> wrote: >> > >> > On Wed, 4 Feb 2009, Felipe Contreras wrote: >> > >> >> On Wed, Feb 4, 2009 at 12:53 AM, Johannes Schindelin >> >> <Johannes.Schindelin@xxxxxx> 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 -- 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
