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
10 changes: 9 additions & 1 deletion lua/diffview/api/views/diff/diff_view.lua
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ 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 @@ -67,7 +75,7 @@ function CDiffView:init(opt)
adapter,
self.files,
self.path_args,
self.rev_arg or adapter:rev_to_pretty_string(opt.left, opt.right)
rev_to_panel_name(adapter, self.rev_arg, opt.left, opt.right)
),
}))

Expand Down
20 changes: 14 additions & 6 deletions lua/diffview/scene/views/diff/diff_view.lua
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ 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 @@ -76,7 +84,7 @@ function DiffView:init(opt)
self.adapter,
self.files,
self.path_args,
self.rev_arg or self.adapter:rev_to_pretty_string(self.left, self.right)
rev_to_panel_name(self.adapter, self.rev_arg, self.left, self.right)
),
})

Expand Down Expand Up @@ -399,7 +407,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 = new_rev_arg
self.panel.rev_pretty_name = rev_to_panel_name(self.adapter, 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 @@ -707,10 +715,6 @@ local update_files_impl = debounce.debounce_trailing(
if new_left and new_right then
self.left = new_left
self.right = new_right

if not self.rev_arg then
self.panel.rev_pretty_name = self.adapter:rev_to_pretty_string(self.left, self.right)
end
end
perf:lap("refreshed revs")

Expand All @@ -728,6 +732,10 @@ 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)
perf:lap("updated rev label")

local index_stat = pl:stat(pl:join(self.adapter.ctx.dir, "index"))

---@type string[]?, FileDict
Expand Down
68 changes: 68 additions & 0 deletions lua/diffview/tests/functional/jj_adapter_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,74 @@ describe("diffview.vcs.adapters.jj", function()
end
end)

describe("rev_to_panel_name", function()
it("includes the description for a single-change range", 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", "update file" })

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

eq(left:abbrev() .. ".." .. right:abbrev(), adapter:rev_to_pretty_string(left, right))
eq(
"update file",
adapter:rev_to_panel_name(left:abbrev() .. ".." .. right:abbrev(), left, right)
)

local non_parent = adapter.Rev(RevType.COMMIT, string.rep("1", 40))
eq("main..@", adapter:rev_to_panel_name("main..@", non_parent, right))
end)

it("uses the current change description for a default no-arg diff", 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", "current 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))
end)

it("falls back for null-tree ranges", function()
if not jj_available() then
pending("jj not installed")
return
end

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

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

eq("root range", adapter:rev_to_panel_name("root range", left, right))
end)
end)

describe("tracked_files", function()
it(
"lists modified, added, and deleted files",
Expand Down
20 changes: 20 additions & 0 deletions lua/diffview/tests/functional/set_revs_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,26 @@ describe("DiffView:set_revs", function()
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)
end)

it("does nothing when parse_revs fails", function()
local left = make_rev("aaa111")
local right = make_rev("bbb222")
Expand Down
12 changes: 12 additions & 0 deletions lua/diffview/vcs/adapter.lua
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,18 @@ function VCSAdapter:rev_to_pretty_string(left, right)
return nil
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.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have backticks around rev_to_pretty_string and rev_arg.

---The default prefers the user's rev_arg; adapters may substitute a
---friendlier label.
---@param rev_arg string?
---@param left Rev
---@param right Rev
---@return string|nil
function VCSAdapter:rev_to_panel_name(rev_arg, left, right)
return rev_arg or self:rev_to_pretty_string(left, right)
end

---Check if any of the given revs are LOCAL.
---@param left Rev
---@param right Rev
Expand Down
66 changes: 66 additions & 0 deletions lua/diffview/vcs/adapters/jj/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,72 @@ function JjAdapter:parse_revs(rev_arg, opt)
return left, right
end

---@param left Rev
---@param right Rev
---@return string|nil
function JjAdapter:single_change_subject(left, right)
if not (left and left.commit and right) then
return
end

if left.commit == JjRev.NULL_TREE_SHA then

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drops the initial commit description. Is this intentional?

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
return
end

local out, code = self:exec_sync(
utils.vec_join(
self:args(),
"--ignore-working-copy",
"log",
"--no-graph",
"-r",
right_rev,
"-T",
[[parents.first().commit_id() ++ "\x1f" ++ description.first_line() ++ "\n"]]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fragile. It's depending the literal string \x1f being interpreted by jj in its template strings. Why not just use \n? Why leak a control character into the label which then has to be stripped out (line 610)? I think the way you've done this enables an invisible glyph to accidentally be copied by a user, which can lead to mysterious bugs.

),
{
cwd = self.ctx.toplevel,
retry = 2,
silent = true,
log_opt = { label = "JjAdapter:single_change_subject()" },
}
)

if code ~= 0 or not out[1] then

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't out itself be nil, or out[1] be ""?

return
end

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

subject = vim.trim(subject or "")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subject can't be nil here (because return would've happened on line 612).

return subject ~= "" and subject or nil
end

---@param rev_arg string?
---@param left Rev
---@param right Rev
---@return string|nil
function JjAdapter:rev_to_panel_name(rev_arg, left, right)
local subject = self:single_change_subject(left, right)
if subject then
return subject
end

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

---@param rev_arg string?
---@param left Rev
---@param right Rev
Expand Down
Loading