git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] replace --edit: respect core.editor

From
Jeff King <peff@peff.net>
Date
Apr 20, 2016, 03:53 UTC
Message-ID
<20160420035353.GA31890@sigill.intra.peff.net>
In-Reply-To
<xmqqoa9536wi.fsf@gitster.mtv.corp.google.com>
On Tue, Apr 19, 2016 at 09:22:37AM -0700, Junio C Hamano wrote:
Show 32 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > We simply need to read the config, is all.
> >
> > This fixes https://github.com/git-for-windows/git/issues/733
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >  builtin/replace.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/builtin/replace.c b/builtin/replace.c
> > index 748c6ca..02b13f6 100644
> > --- a/builtin/replace.c
> > +++ b/builtin/replace.c
> > @@ -475,6 +475,7 @@ int cmd_replace(int argc, const char **argv, const char *prefix)
> >  		return replace_object(argv[0], argv[1], force);
> >  
> >  	case MODE_EDIT:
> > +		git_config(git_default_config, NULL);
> >  		if (argc != 1)
> >  			usage_msg_opt("-e needs exactly one argument",
> >  				      git_replace_usage, options);
> 
> The placement of git_config() makes me wonder why.
> 
> I can understand "we only know edit mode needs config, and we know
> it will never affect other modes to have the new call here", and it
> would be good for an emergency patch for ancient maintenance track
> that will not get any other changes or enhancements.  I do not think
> it is a sound reasoning to maintain the codefor the longer term,
> though.

Yeah. I agree the patch here is not wrong, but I would prefer to just have git-replace load the config when it starts. It's _possible_ that something might break or misbehave, but IMHO any program which breaks when git_default_config() is run is probably in need of fixing.

And I cannot recall any reason we did not read config when "--edit" was added; we just didn't think of it.

-Peff
Previous: Junio C HamanoNext: Christian Couder
Message 3 of 10 in “replace --edit: respect core.editor”
  1. replace --edit: respect core.editorJohannes Schindelin, Apr 19, 2016
  2. Junio C HamanoApr 19, 2016
  3. Jeff KingApr 20, 2016
  4. Christian CouderApr 20, 2016
  5. Johannes SchindelinApr 20, 2016
  6. Johannes SchindelinApr 20, 2016
  7. Junio C HamanoApr 20, 2016
  8. replace --edit: respect core.editorJohannes Schindelin, Apr 20, 2016
  9. Jeff KingApr 20, 2016
  10. Junio C HamanoApr 20, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.