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

Re: [StGit PATCH] Add automatic git-mergetool invocation to the new infrastructure

From
Karl Hasselström <kha@treskal.com>
Date
Feb 11, 2009, 13:11 UTC
Message-ID
<20090211131116.GA30272@diana.vm.bytemark.co.uk>
In-Reply-To
<b0943d9e0902110248n7aa14743p19079e3d967f77a9@mail.gmail.com>
On 2009-02-11 10:48:00 +0000, Catalin Marinas wrote:
Show 31 quoted lines
> 2009/2/11 Karl Hasselström <kha@treskal.com>:
>
> > On 2009-02-10 14:14:07 +0000, Catalin Marinas wrote:
> >
> > > I'm still not entirely sure where the check for stgit.autoimerge
> > > should be done. In the classic infrastructure, it is done in the
> > > merge function. With this patch, it is done in
> > > Transaction.push(). Should we push this even further to
> > > stgit.commands.push? My opinion is not since by having it in
> > > Transaction we get the advantage not listing the conflicts if
> > > the mergetool succeeds and we don't need to abort the
> > > transaction.
> >
> > Yes, one advantage of having it here is that if the user resolves
> > the conflict, we can just continue. I'm not sure I personally like
> > that mode of operation -- you might have guessed by now that I
> > like noninteractive mechanisms -- but I can see how it's useful to
> > someone who does.
>
> I find it useful when I prepare a kernel release and pick patches
> from many branches, it saves some typing with having to run the
> mergetool and restart the pick or push command. It's also useful for
> "sync".
>
> > Another advantage of having it here is that it automatically just
> > works for all commands, not just "stg push".
>
> It works for commands that use Transaction.push_patch(). Other
> commands that use IndexAndWorktree.merge() via some other function
> would not work. Will there be such functions? I suspect a "sync"
> implementation would need additional support in Transaction.
Yes, you're right.
> Any thoughts on calling mergetool from IndexAndWorktree.merge()
> (with an additional parameter to explicitly enable this rather than
> just reading the config option)?

That could very well be a good idea -- I can't think of anything wrong with it. (And it's a good idea to make this a parameter rather than making it read the config option.)

Show 10 quoted lines
> > The disadvantage that I see is that we ask the user to put work
> > into resolving conflicts before we've made sure that we won't roll
> > back the whole transaction. If this is to become a dependable
> > feature, we need a way to make sure we'll never throw away the
> > user's work.
>
> Maybe push_patch() can receive a parameter on whether to invoke
> mergetool. The calling code should know the behaviour for aborting
> transactions and only ask for interactivity if the command is
> expected to leave conflicts in the index.
That sounds like a plan.
-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
Previous: Catalin Marinas
Message 4 of 4 in “Add automatic git-mergetool invocation to the new infrastructure”
  1. Add automatic git-mergetool invocation to the new infrastructureCatalin Marinas, Feb 10, 2009
  2. Karl HasselströmFeb 11, 2009
  3. Catalin MarinasFeb 11, 2009
  4. Karl HasselströmFeb 11, 2009

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.