Skip to content

feat(symlink_target): show relative path if in cwd #1609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elohmeier
Copy link

Before:
CleanShot 2024-11-26 at 04 19 33

After:
CleanShot 2024-11-26 at 04 18 39

@elohmeier
Copy link
Author

Added handling of relative parent dirs:
CleanShot 2024-11-26 at 05 02 38

@pynappo
Copy link
Collaborator

pynappo commented Dec 8, 2024

I think this behavior is probably better than the neo-tree default, but symlinks can be absolute or relative:

image

so I think it'd be better if this was an option. something like:

symlink_target = {
  enabled = true,
  display_targets = "force_relative_within_cwd" -- this pr (or maybe just "force_relative")
  -- "force_absolute" -- current neo-tree behavior
  -- "auto" -- uses vim.loop.fs_readlink(node.path) to display relative/absolute ones respectively
}

Technically I think node.link_to should be derived from fs_readlink instead of fs_realpath as it is right now, but that might be a breaking change.

Copy link
Collaborator

@pynappo pynappo left a comment

Choose a reason for hiding this comment

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

Minor style/correctness concerns:

@@ -528,8 +573,9 @@ end

M.symlink_target = function(config, node, state)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you could move the local methods to just above symlink_target so it's more organized.

Comment on lines +25 to +26
original_path = path:new(original_path):make_relative(".")
reference_path = path:new(reference_path):make_relative(".")
Copy link
Collaborator

@pynappo pynappo Dec 9, 2024

Choose a reason for hiding this comment

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

The paths are already absolute before you pass them in, so this seems unneeded (alternatively, you could make them absolute here, and then not bother having to make them absolute before passing them in)

Suggested change
original_path = path:new(original_path):make_relative(".")
reference_path = path:new(reference_path):make_relative(".")

Comment on lines +36 to +50
local result = ""
for i, ref_parent in ipairs(ref_parents) do
for j, par in ipairs(parents) do
if ref_parent == par then
if i == 1 and j == 1 then
return ""
end

result = result .. table.concat(path_elements, "/", #path_elements - j + 2)
return result
end
end

result = "../" .. result
end
Copy link
Collaborator

@pynappo pynappo Dec 9, 2024

Choose a reason for hiding this comment

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

This might be a slightly faster algorithm, although I haven't tested it much, feel free to ignore:

Suggested change
local result = ""
for i, ref_parent in ipairs(ref_parents) do
for j, par in ipairs(parents) do
if ref_parent == par then
if i == 1 and j == 1 then
return ""
end
result = result .. table.concat(path_elements, "/", #path_elements - j + 2)
return result
end
end
result = "../" .. result
end
if o_path.path.root ~= ref_path.path.root then
return o_path:absolute()
end
local i, j = #parents, #ref_parents
-- find the deepest common directory
while parents[i] == ref_parents[j] and math.min(j, i) > 0 do
i = i - 1
j = j - 1
end
local common_dir = parents[i + 1]
local steps_out = string.rep(".." .. sep, j)
return steps_out .. original_path:sub(#common_dir + 2)

local parents = o_path:parents()
local ref_parents = ref_path:parents()

local path_elements = vim.split(o_path.filename, "/")
Copy link
Collaborator

@pynappo pynappo Dec 9, 2024

Choose a reason for hiding this comment

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

Windows uses backslashes, so this might not always work. to take that into account:

Suggested change
local path_elements = vim.split(o_path.filename, "/")
local sep = o_path.path.sep
local path_elements = vim.split(o_path.filename, sep)

Comment on lines +18 to +28
local path = require("plenary.path")

local M = {}

-- Workaround for https://github.com/nvim-lua/plenary.nvim/issues/411
local relative_path = function(original_path, reference_path)
-- Use plenary's make_relative to clean paths
original_path = path:new(original_path):make_relative(".")
reference_path = path:new(reference_path):make_relative(".")
local o_path = path:new(original_path)
local ref_path = path:new(reference_path)
Copy link
Collaborator

Choose a reason for hiding this comment

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

path from plenary is capitalized in other modules:

Suggested change
local path = require("plenary.path")
local M = {}
-- Workaround for https://github.com/nvim-lua/plenary.nvim/issues/411
local relative_path = function(original_path, reference_path)
-- Use plenary's make_relative to clean paths
original_path = path:new(original_path):make_relative(".")
reference_path = path:new(reference_path):make_relative(".")
local o_path = path:new(original_path)
local ref_path = path:new(reference_path)
local Path = require("plenary.path")
local M = {}
-- Workaround for https://github.com/nvim-lua/plenary.nvim/issues/411
local relative_path = function(original_path, reference_path)
-- Use plenary's make_relative to clean paths
original_path = Path:new(original_path):make_relative(".")
reference_path = Path:new(reference_path):make_relative(".")
local o_path = Path:new(original_path)
local ref_path = Path:new(reference_path)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants