| Submitter | Matt Harbison |
|---|---|
| Date | April 16, 2014, 4:45 a.m. |
| Message ID | <c84cafbba40646fd586f.1397623518@Envy> |
| Download | mbox | patch |
| Permalink | /patch/4383/ |
| State | Deferred |
| Headers | show |
Comments
On Wed, 16 Apr 2014 00:45:18 -0400, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1395456880 14400 > # Node ID c84cafbba40646fd586f6540599963ef3f9046f5 > # Parent 3c598f1d1b79671030d3e6a550dd8727f3429eb9 > cat: use the topmost repo when expanding node based format strings for output This patch is a resend of the original, but with the commit message modified- even without the first patch in this series, the format rules that this fixes were (sort of) documented by referencing export's rule list. That was my misreading of the documentation, and my wrong statement that made Siddharth originally question the value of this. --Matt
On Wed, 16 Apr 2014 04:57:03 +0000, Matt Harbison wrote: > On Wed, 16 Apr 2014 00:45:18 -0400, Matt Harbison wrote: > >> # HG changeset patch >> # User Matt Harbison <matt_harbison@yahoo.com> >> # Date 1395456880 14400 >> # Node ID c84cafbba40646fd586f6540599963ef3f9046f5 >> # Parent 3c598f1d1b79671030d3e6a550dd8727f3429eb9 >> cat: use the topmost repo when expanding node based format strings for output > > This patch is a resend of the original, but with the commit message > modified- even without the first patch in this series, the format rules > that this fixes were (sort of) documented by referencing export's rule > list. That was my misreading of the documentation, and my wrong statement > that made Siddharth originally question the value of this. > > --Matt For context, the original series is here [1]. Siddharth queued the first two, which are required for the second one in this series, but I haven't seen them show up yet. --Matt [1] http://www.selenic.com/pipermail/mercurial-devel/2014-March/057287.html
On Wed, 2014-04-16 at 00:45 -0400, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison <matt_harbison@yahoo.com> > # Date 1395456880 14400 > # Node ID c84cafbba40646fd586f6540599963ef3f9046f5 > # Parent 3c598f1d1b79671030d3e6a550dd8727f3429eb9 > cat: use the topmost repo when expanding node based format strings for output > > Expanding these variables based on the top level repo makes the subrepo seem > more integrated with the parent, and is probably less surprising to a user that > executes this command from the top level repo. > > This solution isn't very satisfying because it requires passing two contexts > around, and it isn't reusable in other places that may need info about the top > level context that includes the repo. The alternate attempt I made was to store > the parent context in the subrepo class when it is initialized. The svn and git > subrepos already do this. That didn't work because the chain that can be > followed up to the root of hgsubrepo is based on localrepository objects, not > subrepo instances. But at least this allows for proper behavior until something > better can be figured out. Yes, this is a bit painful. We've got a way to work back to the root repository object (subrepo._parentrepo), but we don't have a way to pass the relevant hash around. One hack would be to pass it in opts. Or we could extend ctx so that when we create a subrepo context, we can find the parent context. This might be all-around cleaner. I think we should revision this post-3.0.
Patch
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -1812,11 +1812,11 @@ forgot.extend(forget) return bad, forgot -def cat(ui, repo, ctx, matcher, prefix, **opts): +def cat(ui, repo, ctx, matcher, prefix, rootctx, **opts): err = 1 def write(path): - fp = makefileobj(repo, opts.get('output'), ctx.node(), + fp = makefileobj(rootctx._repo, opts.get('output'), rootctx.node(), pathname=os.path.join(prefix, path)) data = ctx[path].data() if opts.get('decode'): @@ -1857,7 +1857,7 @@ submatch = matchmod.narrowmatcher(subpath, matcher) if not sub.cat(ui, submatch, os.path.join(prefix, sub._path), - **opts): + rootctx, **opts): err = 0 except error.RepoLookupError: ui.status(_("skipping missing subrepository: %s\n") diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -1179,7 +1179,7 @@ ctx = scmutil.revsingle(repo, opts.get('rev')) m = scmutil.match(ctx, (file1,) + pats, opts) - return cmdutil.cat(ui, repo, ctx, m, '', **opts) + return cmdutil.cat(ui, repo, ctx, m, '', ctx, **opts) @command('^clone', [('U', 'noupdate', None, diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py --- a/mercurial/subrepo.py +++ b/mercurial/subrepo.py @@ -439,7 +439,7 @@ def add(self, ui, match, dryrun, listsubrepos, prefix, explicitonly): return [] - def cat(self, ui, match, prefix, **opts): + def cat(self, ui, match, prefix, rootctx, **opts): return 1 def status(self, rev2, **opts): @@ -612,10 +612,10 @@ os.path.join(prefix, self._path), explicitonly) @annotatesubrepoerror - def cat(self, ui, match, prefix, **opts): + def cat(self, ui, match, prefix, rootctx, **opts): rev = self._state[1] ctx = self._repo[rev] - return cmdutil.cat(ui, self._repo, ctx, match, prefix, **opts) + return cmdutil.cat(ui, self._repo, ctx, match, prefix, rootctx, **opts) @annotatesubrepoerror def status(self, rev2, **opts): diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t --- a/tests/test-subrepo.t +++ b/tests/test-subrepo.t @@ -796,7 +796,23 @@ test test $ mkdir -p tmp/sub/repo + $ hg cat --output tmp/HH_%H sub/repo/foo + $ hg cat --output tmp/RR_%R sub/repo/foo + $ hg cat --output tmp/h_%h sub/repo/foo + $ hg cat --output tmp/r_%r sub/repo/foo + $ hg cat --output tmp/%s_s sub/repo/foo + $ hg cat --output tmp/%d_d sub/repo/foo $ hg cat -r 0 --output tmp/%p_p sub/repo/foo + $ hg log -r . --template "{rev}: {node|short}\n" + 1: be5eb94e7215 + $ find tmp -type f | sort + tmp/HH_be5eb94e72150f9a8f9be0819fc35d935cbd2f30 + tmp/RR_1 + tmp/foo_s + tmp/h_be5eb94e7215 + tmp/r_1 + tmp/sub/repo/foo_p + tmp/sub/repo_d $ cat tmp/sub/repo/foo_p test $ mv sub/repo sub_