Congratulations on your first patch to the list! But I think we have better ways to achieve the same goal that we may prefer them to this patch.
1) Better way to provide conflicted file contents - in-python merge tools From a high-level, the patch tries to solve the problem: - Get all paths and file contents involved in merge conflict resolution in an efficient way. We have "--list" and "--tool" already to fetch the data in a less efficient way. I'm not a big fan of a new flag doing what we can do today. I think a better to achieve the "efficient" goal is to make "--tool" accept Python functions, just like what we do for hooks. If the signature of the Python function accepts file contents directly, we can even avoid writing temporary files - even more efficient than this patch. 2) Better "-T json" - templates I think it's a bit hacky to only support JSON in a hard-coded format. Ideally I think we want a very-flexible template version. The template seems to fit with "--list" (so we don't need a new flag), and it may look like: - --list -T '{status} {path}\n' # something similar to what we have today - --list -T '... {patch-local}\n{diff-other}\n' # show patches, code reading the output could read the working copy and # apply patches to reconstruct file contents - --list -T '... {temp-path-local}\n{temp-path-other}\n' # create temporary files only when requested - --list -T json:field1,field2,.... # choose what field needed for the JSON output Although this looks promising, this project seems too big. I guess "1)" is a reasonable way to go. In additional, I think "hg resolve" needs to provide a way to not rewrite unresolved files in the working copy. It could be done via a "--dry-run"-alike flag, which is probably a separate patch. Excerpts from Phil Cohen's message of 2017-02-22 22:49:06 -0800: > # HG changeset patch > # User Phil Cohen <phil...@fb.com> > # Date 1487831905 28800 > # Wed Feb 22 22:38:25 2017 -0800 > # Node ID 77da232f2d689cdb9545e060405e8776f2193488 > # Parent 96eaefd350aec869047d9e2da90913ae698463df > merge: add resolve --prep, which outputs conflicted comparison files > > Normally when resolving merge conflicts, `hg resolve` loops over each > specified > file, generating .orig, ~other.xxxxxx, and ~base.xxxxxx files and then > launching > the user's editor (along with a path to the output file with the conflict > markers) and waiting for it to complete. > > We'd like to enable random-access to the list of the conflicted files so the > user can flip more easily between conflicted files. This commit introduces > `resolve --prep`, which generates all of these files up-front and outputs the > result as JSON, which could be consumed by an IDE. (We’re not sure if it makes > sense to have a human readable version yet, so we’re leaving that > functionality > out until a use case demands it.) The intention is that the user will fix all > the conflicts and then run `resolve --mark`. > > Unlike the existing flow, which writes these files to a temporary directory, > these files are stored in `.hg/merge/prep` and get deleted when `.hg/merge` > does. Like the old flow, they have a randomized portion of the filename to > prevent collisions. Technically each call to `resolve --prep` will generate a > new set of files but we consider the cost of this to be low. > > No change is made to the existing merge flow as we decided it was not worth > touching the merge state to reuse the same files. > > diff --git a/mercurial/commands.py b/mercurial/commands.py > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -33,6 +33,7 @@ > error, > exchange, > extensions, > + formatter, > graphmod, > hbisect, > help, > @@ -4241,7 +4242,8 @@ > ('l', 'list', None, _('list state of files needing merge')), > ('m', 'mark', None, _('mark files as resolved')), > ('u', 'unmark', None, _('mark files as unresolved')), > - ('n', 'no-status', None, _('hide status prefix'))] > + ('n', 'no-status', None, _('hide status prefix')), > + ('prep', 'prep', None, _('lists paths to comparison file paths'))] The description reads a bit strange to me. But maybe not to a native speaker. > + mergetoolopts + walkopts + formatteropts, > _('[OPTION]... [FILE]...'), > inferrepo=True) > @@ -4287,8 +4289,8 @@ > Returns 0 on success, 1 if any files fail a resolve attempt. > """ > > - flaglist = 'all mark unmark list no_status'.split() > - all, mark, unmark, show, nostatus = \ > + flaglist = 'all mark unmark list no_status prep'.split() > + all, mark, unmark, show, nostatus, prep = \ > [opts.get(o) for o in flaglist] > > if (show and (mark or unmark)) or (mark and unmark): > @@ -4315,6 +4317,28 @@ > fm.end() > return 0 > > + if prep: > + fm = ui.formatter('resolve', opts) > + if not isinstance(fm, formatter.jsonformatter): > + raise error.Abort(_('--prep requires `-T json`')) This is the first sub-commend that only works with "-T json". So it's not terminal-user-faceing. Maybe a debug command, or change the flag description to hide it, by adding "(ADVANCED)", or "(EXPERIMENTAL)". > + ms = mergemod.mergestate.read(repo) > + m = scmutil.match(repo[None], pats, opts) > + wctx = repo[None] > + > + paths = {} > + for f in ms: > + if not m(f): > + continue > + > + val = ms.prep(f, wctx) > + if val is not None: > + paths[f] = val > + > + fm.startitem() > + fm.write('conflicts', '%s\n', paths) > + fm.end() > + return 0 > + > with repo.wlock(): > ms = mergemod.mergestate.read(repo) > > diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py > --- a/mercurial/filemerge.py > +++ b/mercurial/filemerge.py > @@ -567,6 +567,35 @@ > "o": " [%s]" % labels[1], > } > > +# If repo_dir is None, a temp dir is used > +def temp(prefix, ctx, repo, repo_dir=None): > + fullbase, ext = os.path.splitext(ctx.path()) > + pre = "%s~%s." % (os.path.basename(fullbase), prefix) > + data = repo.wwritedata(ctx.path(), ctx.data()) > + > + if repo_dir: > + repo.vfs.makedirs(repo_dir) > + (fd, name) = repo.vfs.mkstemp(prefix=pre, suffix=ext, dir=repo_dir) > + f = repo.vfs(name, pycompat.sysstr("wb")) > + else: > + (fd, name) = tempfile.mkstemp(prefix=pre, suffix=ext) > + f = os.fdopen(fd, pycompat.sysstr("wb")) > + > + f.write(data) > + f.close() > + return repo.vfs.join(name) > + > +def gentempfiles(repo, fcd, fco, fca): > + back = None if fcd.isabsent() else \ > + scmutil.origpath(repo.ui, repo, repo.wjoin(fcd.path())) > + > + return { > + 'workingcopy': repo.wjoin(fcd.path()), > + 'base': temp("base", fca, repo, "merge/prep"), > + 'other': temp("other", fco, repo, "merge/prep"), > + 'original': back > + } > + > def _filemerge(premerge, repo, mynode, orig, fcd, fco, fca, labels=None): > """perform a 3-way merge in the working directory > > @@ -580,16 +609,6 @@ > Returns whether the merge is complete, the return value of the merge, and > a boolean indicating whether the file was deleted from disk.""" > > - def temp(prefix, ctx): > - fullbase, ext = os.path.splitext(ctx.path()) > - pre = "%s~%s." % (os.path.basename(fullbase), prefix) > - (fd, name) = tempfile.mkstemp(prefix=pre, suffix=ext) > - data = repo.wwritedata(ctx.path(), ctx.data()) > - f = os.fdopen(fd, pycompat.sysstr("wb")) > - f.write(data) > - f.close() > - return name > - Usually the practice is to prefer small patches [1]. Like moving a function could be a single patch. That'll make review easier. [1] https://www.mercurial-scm.org/wiki/ContributingChanges > if not fco.cmp(fcd): # files identical? > return True, None, False > > @@ -637,8 +656,8 @@ > return True, 1, False > > a = repo.wjoin(fd) > - b = temp("base", fca) > - c = temp("other", fco) > + b = temp("base", fca, repo) > + c = temp("other", fco, repo) > if not fcd.isabsent(): > back = scmutil.origpath(ui, repo, a) > if premerge: > diff --git a/mercurial/merge.py b/mercurial/merge.py > --- a/mercurial/merge.py > +++ b/mercurial/merge.py > @@ -456,6 +456,24 @@ > def extras(self, filename): > return self._stateextras.setdefault(filename, {}) > > + def _prep(self, dfile, wctx): > + if self[dfile] in 'rd': > + return None > + stateentry = self._state[dfile] > + state, hash, lfile, afile, anode, ofile, onode, flags = stateentry > + octx = self._repo[self._other] > + extras = self.extras(dfile) > + anccommitnode = extras.get('ancestorlinknode') > + if anccommitnode: > + actx = self._repo[anccommitnode] > + else: > + actx = None > + fcd = self._filectxorabsent(hash, wctx, dfile) > + fco = self._filectxorabsent(onode, octx, ofile) > + fca = self._repo.filectx(afile, fileid=anode, changeid=actx) > + > + return filemerge.gentempfiles(self._repo, fcd, fco, fca) > + > def _resolve(self, preresolve, dfile, wctx): > """rerun merge process for file path `dfile`""" > if self[dfile] in 'rd': > @@ -543,6 +561,9 @@ > Returns whether the merge is complete, and the exit code.""" > return self._resolve(True, dfile, wctx) > > + def prep(self, dfile, wctx): > + return self._prep(dfile, wctx) > + > def resolve(self, dfile, wctx): > """run merge process (assuming premerge was run) for dfile > > diff --git a/tests/test-resolve-prep.t b/tests/test-resolve-prep.t > new file mode 100644 > --- /dev/null > +++ b/tests/test-resolve-prep.t > @@ -0,0 +1,83 @@ > +1) Make the repo > + $ hg init > + > +2) Can't run prep outside a conflict > + $ hg resolve --prep -T json > + abort: no files or directories specified > + (use --all to re-merge all unresolved files) > + [255] > + > +3) Make a simple conflict > + $ echo "Unconflicted base, F1" > F1 > + $ echo "Unconflicted base, F2" > F2 > + $ hg add . > + adding F1 > + adding F2 > + $ hg commit -m "initial commit" > + $ echo "First conflicted version, F1" > F1 > + $ echo "First conflicted version, F2" > F2 > + $ hg commit -m "first version, a" > + $ hg bookmark a > + $ hg checkout .~1 > + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved > + (leaving bookmark a) > + $ echo "Second conflicted version, F1" > F1 > + $ echo "Second conflicted version, F2" > F2 > + $ hg commit -m "second version, b" > + created new head > + $ hg bookmark b > + $ hg log -G -T '({rev}) {desc}\nbookmark: {bookmarks}\nfiles: {files}\n\n' > + @ (2) second version, b > + | bookmark: b > + | files: F1 F2 > + | > + | o (1) first version, a > + |/ bookmark: a > + | files: F1 F2 > + | > + o (0) initial commit > + bookmark: > + files: F1 F2 > + > + > + > + $ hg merge a > + merging F1 > + merging F2 > + warning: conflicts while merging F1! (edit, then use 'hg resolve --mark') > + warning: conflicts while merging F2! (edit, then use 'hg resolve --mark') > + 0 files updated, 0 files merged, 0 files removed, 2 files unresolved > + use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to > abandon > + [1] > + > +4) Must pass '-T json': > + $ hg resolve --prep --all > + abort: --prep requires `-T json` > + [255] > + > +5) Get the paths: > + $ hg resolve --prep --all -T json > + [ > + { > + "conflicts": {"F1": {"base": "$TESTTMP/.hg/merge/prep/F1~base.??????", > "original": "$TESTTMP/F1.orig", "other": > "$TESTTMP/.hg/merge/prep/F1~other.??????", "workingcopy": "$TESTTMP/F1"}, > "F2": {"base": "$TESTTMP/.hg/merge/prep/F2~base.??????", "original": > "$TESTTMP/F2.orig", "other": "$TESTTMP/.hg/merge/prep/F2~other.??????", > "workingcopy": "$TESTTMP/F2"}} (glob) > + } > + ] > + > +6) Ensure the paths point to the right contents: > + $ getpath() { # Usage: getpath <path> <version> > + > local script="import sys, json; print > json.load(sys.stdin)[0][\"conflicts\"][\"$1\"][\"$2\"]" > + > local result=$(hg resolve --prep --all -T json | python -c "$script") > + > echo "$result" > + > } > + $ cat $(getpath "F1" "base") > + Unconflicted base, F1 > + $ cat $(getpath "F1" "other") > + First conflicted version, F1 > + $ cat $(getpath "F1" "original") > + Second conflicted version, F1 > + $ cat $(getpath "F2" "base") > + Unconflicted base, F2 > + $ cat $(getpath "F2" "other") > + First conflicted version, F2 > + $ cat $(getpath "F2" "original") > + Second conflicted version, F2 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel