On Fri, 09 Feb 2018 08:04:04 -0500, Yuya Nishihara <y...@tcha.org> wrote:

On Fri, 09 Feb 2018 00:30:23 -0500, Matt Harbison wrote:
# HG changeset patch
# User Matt Harbison <matt_harbi...@yahoo.com>
# Date 1517098935 18000
#      Sat Jan 27 19:22:15 2018 -0500
# Node ID e545487fa7fa9ff2fa4f85e2482bea8ba4040ef6
# Parent  2b6579d78fe2dde40f8e0e99abeb35df93a4ade5
lfs: teach '{lfs_files}' to handle removed files

diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -360,10 +360,11 @@

 @templatekeyword('lfs_files')
 def lfsfiles(repo, ctx, **args):
- """List of strings. LFS files added or modified by the changeset."""
+    """List of strings. All files modified, added, or removed by this
+    changeset."""
     args = pycompat.byteskwargs(args)

-    pointers = wrapper.pointersfromctx(ctx) # {path: pointer}
+ pointers = wrapper.pointersfromctx(ctx, removed=True) # {path: pointer}
     files = sorted(pointers.keys())

+{lfs_files} will list deleted files too
+
+  $ hg log -T "{lfs_files % '{rev} {file}: {lfspointer.oid}\n'}"
+ 6 lfs.test: sha256:43f8f41171b6f62a6b61ba4ce98a8a6c1649240a47ebafd43120aa215ac9e7f6 + 5 lfs.test: sha256:43f8f41171b6f62a6b61ba4ce98a8a6c1649240a47ebafd43120aa215ac9e7f6 + 3 lfs.catchall: sha256:31f43b9c62b540126b0ad5884dc013d21a61c9329b77de1fceeae2fc58511573 + 3 lfs.test: sha256:8acd23467967bc7b8cc5a280056589b0ba0b17ff21dbd88a7b6474d6290378a6 + 2 lfs.catchall: sha256:d4ec46c2869ba22eceb42a729377432052d9dd75d82fc40390ebaadecee87ee9 + 2 lfs.test: sha256:5489e6ced8c36a7b267292bde9fd5242a5f80a7482e8f23fa0477393dfaa4d6c

Including removed files makes sense, but I think the pointer should be "null" in which case because the file doesn't exist. I have no idea how the "null"
lfs pointer should look like, though.

Interesting thought, I hadn't considered it. I guess was going for being able to identify which file was removed. But that doesn't quite work, because lfs files can transition to non-lfs first.

My only concern would be not complicating the template writing. Having to wrap lfspointer inside an {if()} before accessing lfspointer.foo or lfspointer % oid seems unfortunate. IDK how to represent a null pointer either (internally or externally), so I guess I'll let it sit a few days to see if anyone has an idea. I'm waiting on stable to merge before I can submit a similar patch for set:lfs().

In the meantime, here's a total hatchet job, trying to see what avoiding {if()} looks like in the tests. I can't say I like it either.

diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -375,8 +375,8 @@

     makemap = lambda v: {
         'file': v,
-        'lfsoid': pointers[v].oid(),
-        'lfspointer': templatekw.hybriddict(pointer(v)),
+        'lfsoid': pointers[v].oid() if pointers[v].get('oid') else None,
+        'lfspointer': templatekw.hybriddict(pointer(v))
     }

     # TODO: make the separator ', '?
diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py
--- a/hgext/lfs/wrapper.py
+++ b/hgext/lfs/wrapper.py
@@ -375,8 +375,12 @@
     result = {}
     for f in ctx.files():
         p = pointerfromctx(ctx, f, removed=removed)
+        p2 = pointerfromctx(ctx, f)
         if p:
-            result[f] = p
+            if p2:
+                result[f] = p
+            else:
+                result[f] = {k: None for k, v in p.iteritems()}
     return result

 def uploadblobs(repo, pointers):
diff --git a/tests/test-lfs.t b/tests/test-lfs.t
--- a/tests/test-lfs.t
+++ b/tests/test-lfs.t
@@ -1035,13 +1035,21 @@
 {lfs_files} will list deleted files too

   $ hg log -T "{lfs_files % '{rev} {file}: {lfspointer.oid}\n'}"
- 6 lfs.test: sha256:43f8f41171b6f62a6b61ba4ce98a8a6c1649240a47ebafd43120aa215ac9e7f6
+  6 lfs.test:
5 lfs.test: sha256:43f8f41171b6f62a6b61ba4ce98a8a6c1649240a47ebafd43120aa215ac9e7f6 3 lfs.catchall: sha256:31f43b9c62b540126b0ad5884dc013d21a61c9329b77de1fceeae2fc58511573 3 lfs.test: sha256:8acd23467967bc7b8cc5a280056589b0ba0b17ff21dbd88a7b6474d6290378a6 2 lfs.catchall: sha256:d4ec46c2869ba22eceb42a729377432052d9dd75d82fc40390ebaadecee87ee9 2 lfs.test: sha256:5489e6ced8c36a7b267292bde9fd5242a5f80a7482e8f23fa0477393dfaa4d6c

+  $ hg log -T "{lfs_files % '{rev} {file}: {lfspointer}\n'}"
+  6 lfs.test: version=None oid=None size=None x-is-binary=None
+ 5 lfs.test: version=https://git-lfs.github.com/spec/v1 oid=sha256:43f8f41171b6f62a6b61ba4ce98a8a6c1649240a47ebafd43120aa215ac9e7f6 size=25 x-is-binary=0 + 3 lfs.catchall: version=https://git-lfs.github.com/spec/v1 oid=sha256:31f43b9c62b540126b0ad5884dc013d21a61c9329b77de1fceeae2fc58511573 size=19 x-is-binary=0 + 3 lfs.test: version=https://git-lfs.github.com/spec/v1 oid=sha256:8acd23467967bc7b8cc5a280056589b0ba0b17ff21dbd88a7b6474d6290378a6 size=11 x-is-binary=0 + 2 lfs.catchall: version=https://git-lfs.github.com/spec/v1 oid=sha256:d4ec46c2869ba22eceb42a729377432052d9dd75d82fc40390ebaadecee87ee9 size=18 x-is-binary=0 + 2 lfs.test: version=https://git-lfs.github.com/spec/v1 oid=sha256:5489e6ced8c36a7b267292bde9fd5242a5f80a7482e8f23fa0477393dfaa4d6c size=10 x-is-binary=0
+
 TODO: This should notice the deleted lfs files in rev 6
   $ hg log -r 'file("set:lfs()")' -T '{rev} {join(lfs_files, ", ")}\n'
   2 lfs.catchall, lfs.test
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to