From: Catalin Marinas Date: Mon, 16 Mar 2009 15:03:09 GMT Subject: Re: [StGit PATCH 3/5] Add automatic git-mergetool invocation to the new infrastructure Message-ID: In-Reply-To: <20090313021751.GC15393@diana.vm.bytemark.co.uk> 2009/3/13 Karl Hasselström : > On 2009-03-12 12:09:07 +0000, Catalin Marinas wrote: >>                  # There were conflicts >> -                conflicts = [l for l in output if l.startswith('CONFLICT')] >> -                raise MergeConflictException(conflicts) >> +                if interactive: >> +                    self.mergetool() >> +                else: >> +                    conflicts = [l for l in output if l.startswith('CONFLICT')] >> +                    raise MergeConflictException(conflicts) > > Does the merge tool always resolve all conflicts? If it doesn't, the > two lines in the "else" branch should probably be run unconditionally. [...] >> +        # check for unmerged entries (prepend 'CONFLICT ' for consistency with >> +        # merge()) >> +        conflicts = ['CONFLICT ' + f for f in self.index.conflicts()] >> +        if conflicts: >> +            raise MergeConflictException(conflicts) >> +        elif err: >> +            raise MergeException('"git mergetool" failed, exit code: %d' % err) > > Ah, you take care of conflicts here too. Hmm. I guess that's fine too, > though there is some code duplication. Maybe a helper function that > takes output as a parameter, and raises MergeConflictException if > necessary? The non-interactive path assumes that there are conflicts if "git merge-recursive" returned an error and it simply splits the output if this command. The mergetool path has to run "git ls-files --unmerged" to check if there were any left conflicts. I wouldn't call "git ls-files" in the first case as we already have the information. -- Catalin