fix(jj): detect renamed files correctly

lua/diffview/tests/functional/jj_adapter_spec.lua

@@ -774,6 +774,148 @@ describe("diffview.vcs.adapters.jj", function()
         end)
       )
 
+      it(
+        "reports the full old and new paths for a renamed file",
+        helpers.async_test(function()
+          if not jj_available() then
+            pending("jj not installed")
+            return
+          end
+
+          -- Same directory, common suffix (`.lua`) -- this is the shape `jj diff
+          -- --summary` would abbreviate to `src/{main => renamed}.lua`. Content is left
+          -- untouched so jj's similarity heuristic reliably reports this as a rename
+          -- rather than add+delete.
+          repo.write("src/main.lua", 'print("v1")\n')
+          repo.jj({ "describe", "-m", "initial" })
+          repo.jj({ "new" })
+
+          os.rename(repo.dir .. "/src/main.lua", repo.dir .. "/src/renamed.lua")
+
+          local adapter = repo.adapter()
+          local left = adapter.Rev(
+            RevType.COMMIT,
+            run({ "jj", "show", "-T", "commit_id", "@-", "--no-patch" }, repo.dir)
+          )
+          local right = adapter.Rev(RevType.LOCAL)
+          local args = adapter:rev_to_args(left, right)
+
+          local err, files = await(
+            adapter:tracked_files(
+              left,
+              right,
+              args,
+              "working",
+              { default_layout = Diff2, merge_layout = Diff2 }
+            )
+          )
+
+          assert.is_nil(err)
+          assert.equals(1, #files)
+          assert.equals("R", files[1].status)
+          assert.equals("src/renamed.lua", files[1].path)
+          assert.equals("src/main.lua", files[1].oldpath)
+        end)
+      )
+
+      it(
+        "reports the full old and new paths when they contain literal `{`/`}`",
+        helpers.async_test(function()
+          if not jj_available() then
+            pending("jj not installed")
+            return
+          end
+
+          -- Regression coverage for a corner case that trips up a naive
+          -- `--summary` parser: renaming `a{x}/f.txt` to `a{y}/f.txt` renders
+          -- as `R {a{x} => a{y}}/f.txt`, where jj's own wrapping braces sit
+          -- right next to literal braces from the directory names. A parser
+          -- that just matches the first `{`/`}` pair gets this wrong (e.g.
+          -- recovering `a{x}}/f.txt` instead of `a{x}/f.txt`). `-T
+          -- TRACKED_FILES_TEMPLATE` sidesteps the problem entirely by reading
+          -- `source().path()`/`target().path()` directly, so this should
+          -- round-trip correctly regardless of what characters the path
+          -- contains.
+          repo.write("a{x}/f.txt", "hi\n")
+          repo.jj({ "describe", "-m", "add" })
+          repo.jj({ "new" })
+
+          vim.fn.mkdir(repo.dir .. "/a{y}", "p")
+          os.rename(repo.dir .. "/a{x}/f.txt", repo.dir .. "/a{y}/f.txt")
+
+          local adapter = repo.adapter()
+          local left = adapter.Rev(
+            RevType.COMMIT,
+            run({ "jj", "show", "-T", "commit_id", "@-", "--no-patch" }, repo.dir)
+          )
+          local right = adapter.Rev(RevType.LOCAL)
+          local args = adapter:rev_to_args(left, right)
+
+          local err, files = await(
+            adapter:tracked_files(
+              left,
+              right,
+              args,
+              "working",
+              { default_layout = Diff2, merge_layout = Diff2 }
+            )
+          )
+
+          assert.is_nil(err)
+          assert.equals(1, #files)
+          assert.equals("R", files[1].status)
+          assert.equals("a{y}/f.txt", files[1].path)
+          assert.equals("a{x}/f.txt", files[1].oldpath)
+        end)
+      )
+
+      it(
+        "reports the full old and new paths when a name contains ` => `",
+        helpers.async_test(function()
+          if not jj_available() then
+            pending("jj not installed")
+            return
+          end
+
+          -- Unlike the `{`/`}` case above, this one is provably unrecoverable
+          -- from `--summary` output, not just hard to parse: renaming a file
+          -- named `p => q` to `z`, and separately renaming a file named `p`
+          -- to `q => z`, both render as the identical line `R {p => q => z}`
+          -- -- no parser can tell which rename produced it. `-T
+          -- TRACKED_FILES_TEMPLATE` never renders that ambiguous form in the
+          -- first place, so this resolves correctly.
+          repo.write("p => q", "hi\n")
+          repo.jj({ "describe", "-m", "add" })
+          repo.jj({ "new" })
+
+          os.rename(repo.dir .. "/p => q", repo.dir .. "/z")
+
+          local adapter = repo.adapter()
+          local left = adapter.Rev(
+            RevType.COMMIT,
+            run({ "jj", "show", "-T", "commit_id", "@-", "--no-patch" }, repo.dir)
+          )
+          local right = adapter.Rev(RevType.LOCAL)
+          local args = adapter:rev_to_args(left, right)
+
+          local err, files = await(
+            adapter:tracked_files(
+              left,
+              right,
+              args,
+              "working",
+              { default_layout = Diff2, merge_layout = Diff2 }
+            )
+          )
+
+          assert.is_nil(err)
+          assert.equals(1, #files)
+          assert.equals("R", files[1].status)
+          assert.equals("z", files[1].path)
+          assert.equals("p => q", files[1].oldpath)
+        end)
+      )
+
       it(
         "shows file content at a revision without errors",
         helpers.async_test(function()
@@ -1823,6 +1965,47 @@ describe("diffview.vcs.adapters.jj", function()
     end)
   end)
 
+  describe("parse_tracked_files_line", function()
+    local parse_tracked_files_line =
+      require("diffview.vcs.adapters.jj")._test.parse_tracked_files_line
+
+    local US = "\x1f" -- field separator
+
+    it("parses a modified/added/deleted line with no old path", function()
+      local status, path, oldpath = parse_tracked_files_line("M" .. US .. "src/main.lua" .. US)
+      assert.equals("M", status)
+      assert.equals("src/main.lua", path)
+      assert.is_nil(oldpath)
+    end)
+
+    it("parses a renamed line with the old path in the 3rd field", function()
+      local status, path, oldpath =
+        parse_tracked_files_line("R" .. US .. "common/new.txt" .. US .. "common/old.txt")
+      assert.equals("R", status)
+      assert.equals("common/new.txt", path)
+      assert.equals("common/old.txt", oldpath)
+    end)
+
+    it("parses a copied line the same way as renamed", function()
+      local status, path, oldpath = parse_tracked_files_line("C" .. US .. "b.txt" .. US .. "a.txt")
+      assert.equals("C", status)
+      assert.equals("b.txt", path)
+      assert.equals("a.txt", oldpath)
+    end)
+
+    it("treats an empty 3rd field as nil, not an empty string", function()
+      local _, _, oldpath = parse_tracked_files_line("M" .. US .. "f.txt" .. US .. "")
+      assert.is_nil(oldpath)
+    end)
+
+    it("returns nil for a line with no path (e.g. a stray blank line)", function()
+      local status, path, oldpath = parse_tracked_files_line("")
+      assert.is_nil(status)
+      assert.is_nil(path)
+      assert.is_nil(oldpath)
+    end)
+  end)
+
   describe("parse_fh_data", function()
     local Diff2 = require("diffview.scene.layouts.diff_2").Diff2
 

lua/diffview/vcs/adapters/jj/init.lua

@@ -1661,6 +1661,55 @@ JjAdapter._query_merge_context = async.wrap(function(self, callback)
   callback(nil, ctx)
 end)
 
+---Template fed to `jj diff -T ...` by `tracked_files` to list the status,
+---path, and (for renames/copies) old path of each changed file.
+---
+---Each file produces exactly one line terminated by `\n`. Fields are
+---separated by `\x1f` (ASCII US) and are, in order:
+---  1. status_char (`M`, `A`, `D`, `R`, `C`, ...)
+---  2. path (the target/right path -- the only path for non-renames)
+---  3. old path, or an empty string when the entry isn't a rename/copy
+---
+---`-T` is used instead of `--summary`: `--summary`'s built-in rename/copy
+---display (`display_diff_path()`) factors out the common prefix/suffix of
+---the two paths into `prefix{old => new}suffix`, unescaped. That's lossy,
+---not just awkward to parse: renaming `p => q` to `z`, and separately
+---renaming `p` to `q => z`, both render as `R {p => q => z}` -- no parser
+---can recover which happened from that string alone. Calling
+---`self.source().path()` / `self.target().path()` directly instead always
+---yields the full, unabbreviated paths.
+---
+---The old-path field is always present (empty when unused) rather than
+---omitted, so every line has exactly 3 fields and parsing never has to
+---branch on how many separators showed up.
+local TRACKED_FILES_TEMPLATE = table.concat({
+  [[ self.status_char() ++ "\x1f" ++ self.path() ++ "\x1f" ]],
+  [[ ++ if(self.status() == "renamed" || self.status() == "copied", ]],
+  [[      self.source().path(), "") ]],
+  [[ ++ "\n" ]],
+}, "")
+
+---Parse one line of `TRACKED_FILES_TEMPLATE` output.
+---
+---Returns `nil` for a line with no path (e.g. a stray blank line), so
+---callers can `if not status then goto continue end`-style skip it.
+---@param line string
+---@return string? status
+---@return string? path
+---@return string? oldpath
+local function parse_tracked_files_line(line)
+  local fields = vim.split(line, "\x1f", { plain = true })
+  local status, path = fields[1], fields[2]
+
+  if not status or not path or path == "" then
+    return nil
+  end
+
+  local oldpath = fields[3] ~= "" and fields[3] or nil
+
+  return status, path, oldpath
+end
+
 ---@param self JjAdapter
 ---@param left Rev
 ---@param right Rev
@@ -1671,7 +1720,7 @@ end)
 JjAdapter.tracked_files = async.wrap(function(self, left, right, args, kind, opt, callback)
   local job = Job({
     command = self:bin(),
-    args = utils.vec_join(self:args(), "diff", "--summary", args),
+    args = utils.vec_join(self:args(), "diff", "-T", TRACKED_FILES_TEMPLATE, args),
     cwd = self.ctx.toplevel,
     retry = 2,
     log_opt = { label = "JjAdapter:tracked_files()" },
@@ -1706,27 +1755,18 @@ JjAdapter.tracked_files = async.wrap(function(self, left, right, args, kind, opt
 
   local files = {}
   for _, line in ipairs(job.stdout) do
-    local status, path = line:match("^(%u)%s+(.*)$")
-
-    if status and path then
-      local oldpath
-      if status == "R" or status == "C" then
-        local from_path, to_path = path:match("^(.-)%s+=>%s+(.-)$")
-        oldpath = from_path
-        path = to_path or path
-      end
+    local status, path, oldpath = parse_tracked_files_line(line)
 
-      if not conflicting[path] then
-        files[#files + 1] = FileEntry.with_layout(opt.default_layout, {
-          adapter = self,
-          path = path,
-          oldpath = oldpath,
-          status = status,
-          stats = {},
-          kind = kind,
-          revs = { a = left, b = right },
-        })
-      end
+    if status and path and not conflicting[path] then
+      files[#files + 1] = FileEntry.with_layout(opt.default_layout, {
+        adapter = self,
+        path = path,
+        oldpath = oldpath,
+        status = status,
+        stats = {},
+        kind = kind,
+        revs = { a = left, b = right },
+      })
     end
   end
 
@@ -2002,7 +2042,7 @@ M.JjAdapter = JjAdapter
 -- adapter; the shape is unstable.
 M._test = {
   structure_fh_data = structure_fh_data,
-  FH_TEMPLATE = FH_TEMPLATE,
+  parse_tracked_files_line = parse_tracked_files_line,
   is_non_literal_pathspec = is_non_literal_pathspec,
   is_ambiguous_literal_path = is_ambiguous_literal_path,
   quote_path_args = quote_path_args,