Patchwork [2,of,2] cat: use the topmost repo when expanding node based format strings for output

login
register
mail settings
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

Matt Harbison - April 16, 2014, 4:45 a.m.
# 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.
Matt Harbison - April 16, 2014, 4:57 a.m.
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
Matt Harbison - April 16, 2014, 12:42 p.m.
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
Matt Mackall - April 16, 2014, 11:45 p.m.
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_