Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 1 addition & 18 deletions lua/diffview/api/views/diff/diff_view.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ require("diffview.bootstrap")
local DiffView = lazy.access("diffview.scene.views.diff.diff_view", "DiffView") ---@type DiffView|LazyModule
local File = lazy.access("diffview.vcs.file", "File") ---@type vcs.File|LazyModule
local FileEntry = lazy.access("diffview.scene.file_entry", "FileEntry") ---@type FileEntry|LazyModule
local FilePanel = lazy.access("diffview.scene.views.diff.file_panel", "FilePanel") ---@type FilePanel|LazyModule
local Rev = lazy.access("diffview.vcs.adapters.git.rev", "GitRev") ---@type GitRev|LazyModule
local RevType = lazy.access("diffview.vcs.rev", "RevType") ---@type RevType|LazyModule
local vcs_utils = lazy.require("diffview.vcs") ---@module "diffview.vcs"
Expand All @@ -18,14 +17,6 @@ local logger = DiffviewGlobal.logger

local M = {}

local function rev_to_panel_name(adapter, rev_arg, left, right)
if adapter.rev_to_panel_name then
return adapter:rev_to_panel_name(rev_arg, left, right)
end

return rev_arg or adapter:rev_to_pretty_string(left, right)
end

---@class FileData
---@field path string Path relative to git root.
---@field oldpath string|nil If the file has been renamed, this should be the old path, oterhwise nil.
Expand Down Expand Up @@ -69,15 +60,7 @@ function CDiffView:init(opt)
self.fetch_files = opt.update_files
self.get_file_data = opt.get_file_data

self:super(vim.tbl_extend("force", opt, {
adapter = adapter,
panel = FilePanel(
adapter,
self.files,
self.path_args,
rev_to_panel_name(adapter, self.rev_arg, opt.left, opt.right)
),
}))
self:super(vim.tbl_extend("force", opt, { adapter = adapter }))

if type(opt.files) == "table" and not vim.tbl_isempty(opt.files) then
local files = self:create_file_entries(opt.files)
Expand Down
21 changes: 9 additions & 12 deletions lua/diffview/scene/views/diff/diff_view.lua
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ local M = {}

local same_rev = lazy.access(rev_lib, "same_rev") --[[@as fun(a: Rev?, b: Rev?): boolean ]]

local function rev_to_panel_name(adapter, rev_arg, left, right)
if adapter.rev_to_panel_name then
return adapter:rev_to_panel_name(rev_arg, left, right)
end

return rev_arg or adapter:rev_to_pretty_string(left, right)
end

---@class DiffViewOptions
---@field show_untracked? boolean
---@field selected_file? string Path to the preferred initially selected file.
Expand Down Expand Up @@ -84,7 +76,7 @@ function DiffView:init(opt)
self.adapter,
self.files,
self.path_args,
rev_to_panel_name(self.adapter, self.rev_arg, self.left, self.right)
self.adapter:rev_to_panel_name(self.rev_arg, self.left, self.right)
),
})

Expand Down Expand Up @@ -407,7 +399,7 @@ function DiffView:set_revs(new_rev_arg, opts)
self.rev_arg = new_rev_arg
self.left = new_left
self.right = new_right
self.panel.rev_pretty_name = rev_to_panel_name(self.adapter, new_rev_arg, self.left, self.right)
self.panel.rev_pretty_name = self.adapter:rev_to_panel_name(new_rev_arg, self.left, self.right)

-- Migrate selection persistence to the new scope key.
if self._selection_scope_key then
Expand Down Expand Up @@ -732,8 +724,13 @@ local update_files_impl = debounce.debounce_trailing(
perf:lap("updated head rev")
end

self.panel.rev_pretty_name =
rev_to_panel_name(self.adapter, self.rev_arg, self.left, self.right)
-- Recompute the panel label unconditionally: adapter-resolved metadata
-- (e.g., a jj change description) can shift even when `left`/`right`
-- are structurally unchanged, e.g., `jj describe` on `@` rewrites the
-- description but leaves the LOCAL rev's `object_name()` fixed at
-- "UNKNOWN", so `refresh_revs` returns nil for that case. Placed after
-- the head-rev update so a HEAD move is reflected in the label too.
self.panel.rev_pretty_name = self.adapter:rev_to_panel_name(self.rev_arg, self.left, self.right)
perf:lap("updated rev label")

local index_stat = pl:stat(pl:join(self.adapter.ctx.dir, "index"))
Expand Down
69 changes: 58 additions & 11 deletions lua/diffview/tests/functional/jj_adapter_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -627,46 +627,93 @@ describe("diffview.vcs.adapters.jj", function()
"update file",
adapter:rev_to_panel_name(left:abbrev() .. ".." .. right:abbrev(), left, right)
)
end)

local non_parent = adapter.Rev(RevType.COMMIT, string.rep("1", 40))
eq("main..@", adapter:rev_to_panel_name("main..@", non_parent, right))
it("substitutes the description for a LOCAL right (`:DiffviewOpen`)", function()
if not jj_available() then
pending("jj not installed")
return
end

repo.write("file.txt", "first\n")
repo.jj({ "describe", "-m", "initial" })
repo.jj({ "new" })
repo.write("file.txt", "second\n")
repo.jj({ "describe", "-m", "working copy change" })

local adapter = repo.adapter()
local parent_hash =
run({ "jj", "show", "-T", "parents.first().commit_id()", "@", "--no-patch" }, repo.dir)
local left = adapter.Rev(RevType.COMMIT, parent_hash)
local right = adapter.Rev(RevType.LOCAL)

eq("working copy change", adapter:rev_to_panel_name(nil, left, right))
end)

it("uses the current change description for a default no-arg diff", function()
it("includes the description for an initial-commit diff (root..@)", function()
if not jj_available() then
pending("jj not installed")
return
end

repo.write("file.txt", "first\n")
repo.jj({ "describe", "-m", "the very first change" })

local adapter = repo.adapter()
local commit_hash = run({ "jj", "show", "-T", "commit_id", "@", "--no-patch" }, repo.dir)
-- Initial commit's parent is the synthetic root; its commit_id IS
-- `NULL_TREE_SHA`. The description should still surface.
local left = adapter.Rev(RevType.COMMIT, adapter.Rev.NULL_TREE_SHA)
local right = adapter.Rev(RevType.COMMIT, commit_hash)

eq("the very first change", adapter:rev_to_panel_name(nil, left, right))
end)

it("routes through `parse_revs` for the working-copy `:DiffviewOpen` case", function()
if not jj_available() then
pending("jj not installed")
return
end

-- End-to-end coverage of the primary use case: `parse_revs(nil, {})`
-- returns a LOCAL right whose `object_name()` is `"UNKNOWN"`, so
-- `single_change_subject` must reach jj via the `@` fallback.
repo.write("file.txt", "first\n")
repo.jj({ "describe", "-m", "initial" })
repo.jj({ "new" })
repo.write("file.txt", "second\n")
repo.jj({ "describe", "-m", "current change" })
repo.jj({ "describe", "-m", "working copy change" })

local adapter = repo.adapter()
local left, right = adapter:parse_revs(nil, {})

eq(RevType.COMMIT, left.type)
eq(RevType.LOCAL, right.type)
eq("current change", adapter:rev_to_panel_name(nil, left, right))
eq("working copy change", adapter:rev_to_panel_name(nil, left, right))
end)

it("falls back for null-tree ranges", function()
it("falls back to `rev_arg` when left is not a parent of right", function()
if not jj_available() then
pending("jj not installed")
return
end

-- Three-commit chain so grandparent is a real (non-null-tree) commit.
repo.write("file.txt", "first\n")
repo.jj({ "describe", "-m", "initial" })
repo.jj({ "describe", "-m", "first" })
repo.jj({ "new" })
repo.write("file.txt", "second\n")
repo.jj({ "describe", "-m", "second" })
repo.jj({ "new" })
repo.write("file.txt", "third\n")
repo.jj({ "describe", "-m", "third" })

local adapter = repo.adapter()
local grandparent_hash =
run({ "jj", "show", "-T", "commit_id", "@--", "--no-patch" }, repo.dir)
local commit_hash = run({ "jj", "show", "-T", "commit_id", "@", "--no-patch" }, repo.dir)
local left = adapter.Rev.new_null_tree()
local non_parent = adapter.Rev(RevType.COMMIT, grandparent_hash)
local right = adapter.Rev(RevType.COMMIT, commit_hash)

eq("root range", adapter:rev_to_panel_name("root range", left, right))
eq("main..@", adapter:rev_to_panel_name("main..@", non_parent, right))
end)
end)

Expand Down
32 changes: 11 additions & 21 deletions lua/diffview/tests/functional/set_revs_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ describe("DiffView:set_revs", function()
rev_to_pretty_string = function(_, left, right)
return (left.commit or "?") .. ".." .. (right.commit or "?")
end,
-- Distinctive sentinel that encodes all three inputs so tests catch
-- both a regression that copies raw `rev_arg` into the panel without
-- consulting the adapter, and one that forwards the wrong `left`/
-- `right` (e.g., pre-update values).
rev_to_panel_name = function(self, rev_arg, left, right)
return rev_arg
and ("PANEL[" .. rev_arg .. "|" .. self:rev_to_pretty_string(left, right) .. "]")
end,
instanceof = function()
return false
end,
Expand Down Expand Up @@ -131,27 +139,9 @@ describe("DiffView:set_revs", function()

view:set_revs("ccc333..ddd444")

eq("ccc333..ddd444", view.panel.rev_pretty_name)
end)

it("uses adapter panel labels when available", function()
local old_left = make_rev("aaa111")
local old_right = make_rev("bbb222")
local new_left = make_rev("ccc333")
local new_right = make_rev("ddd444")

local adapter = make_adapter({
["ccc333..ddd444"] = { new_left, new_right },
})
adapter.rev_to_panel_name = function(_, rev_arg, left, right)
return table.concat({ "label", rev_arg, left.commit, right.commit }, ":")
end

local view = make_view(adapter, old_left, old_right, "aaa111..bbb222")

view:set_revs("ccc333..ddd444")

eq("label:ccc333..ddd444:ccc333:ddd444", view.panel.rev_pretty_name)
-- Prove the adapter method is consulted with the NEW `left`/`right`
-- (post-update); the mock's sentinel encodes all three inputs.
eq("PANEL[ccc333..ddd444|ccc333..ddd444]", view.panel.rev_pretty_name)
end)

it("does nothing when parse_revs fails", function()
Expand Down
4 changes: 2 additions & 2 deletions lua/diffview/vcs/adapter.lua
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,8 @@ function VCSAdapter:rev_to_pretty_string(left, right)
end

---Convert revs to the display-only label shown in the file panel.
---Unlike rev_to_pretty_string the result need not be parseable as a rev.
---The default prefers the user's rev_arg; adapters may substitute a
---Unlike `rev_to_pretty_string`, the result need not be parseable as a rev.
---The default prefers the user's `rev_arg`; adapters may substitute a
---friendlier label.
---@param rev_arg string?
---@param left Rev
Expand Down
52 changes: 31 additions & 21 deletions lua/diffview/vcs/adapters/jj/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -576,33 +576,32 @@ end
---@param right Rev
---@return string|nil
function JjAdapter:single_change_subject(left, right)
if not (left and left.commit and right) then
-- Preconditions: `left` is a resolved commit, `right` is either a resolved
-- commit or LOCAL (which spells `@` on the CLI); reject the null-tree
-- sentinel because `jj show` on it emits a template error.
if not left.commit or right.commit == JjRev.NULL_TREE_SHA then
return
end

if left.commit == JjRev.NULL_TREE_SHA then
return
end

local right_rev
if right.type == RevType.LOCAL then
right_rev = "@"
elseif right.commit and right.commit ~= JjRev.NULL_TREE_SHA then
right_rev = right.commit
else
local right_arg = right.commit or (right.type == RevType.LOCAL and "@")
if not right_arg then
return
end

-- Emit `parent_hash\ndescription`. `\n` is officially supported in jj
-- template strings, and `description.first_line()` guarantees no embedded
-- newlines, so the output splits cleanly across two lines. `self:args()`
-- carries any user-configured global flags (e.g., `--repository`) ahead
-- of the subcommand, matching how `get_show_args`/`get_log_args` build
-- their invocations.
local out, code = self:exec_sync(
utils.vec_join(
self:args(),
"--ignore-working-copy",
"log",
"--no-graph",
"-r",
right_rev,
"show",
"--no-patch",
"-T",
[[parents.first().commit_id() ++ "\x1f" ++ description.first_line() ++ "\n"]]
[[parents.first().commit_id() ++ "\n" ++ description.first_line()]],
right_arg
),
{
cwd = self.ctx.toplevel,
Expand All @@ -612,19 +611,30 @@ function JjAdapter:single_change_subject(left, right)
}
)

if code ~= 0 or not out[1] then
-- Explicit `not out` catches the bootstrap-fail path where `exec_sync`
-- returns bare nils; we don't want to rely on `code ~= 0`'s short-circuit
-- to shield the following `out[1]` index.
if code ~= 0 or not out or not out[1] then
return
end

local parent, subject = vim.trim(out[1]):match("^([^\31]*)\31(.*)$")
local parent = vim.trim(out[1])
if parent ~= left.commit then
return
end

subject = vim.trim(subject or "")
return subject ~= "" and subject or nil
local subject = vim.trim(out[2] or "")
if subject == "" then
return
end

return subject
end

---Convert revs to the display-only label shown in the file panel.
---Unlike `rev_to_pretty_string`, the result need not be parseable as a rev.
---For a single-change range, use the change description; otherwise fall
---back to the caller's `rev_arg` or the default pretty string.
---@param rev_arg string?
---@param left Rev
---@param right Rev
Expand Down
Loading