Skip to content
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

feat(core): enhance org-insert-link, add completion for ~/ prefix. #749

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
15 changes: 4 additions & 11 deletions lua/orgmode/org/hyperlinks/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,13 @@ function Hyperlinks.find_by_filepath(url)
return {}
end
--TODO integrate with orgmode.utils.fs or orgmode.objects.url
local file_base_no_start_path = vim.pesc(file_base:gsub('^%./', '') .. '')
local is_relative_path = file_base:match('^%./')
local current_file_directory = vim.pesc(fs.get_current_file_dir())
local valid_filenames = {}
for _, f in ipairs(filenames) do
if is_relative_path then
local match = f:match('^' .. current_file_directory .. '/(' .. file_base_no_start_path .. '.*%.org)$')
if match then
table.insert(valid_filenames, './' .. match)
end
else
if f:find('^' .. file_base) then
table.insert(valid_filenames, f)
if f:find('^' .. file_base) then
if url.realpath then
f = f:gsub(file_base, url.path)
end
table.insert(valid_filenames, f)
end
end

Expand Down
9 changes: 7 additions & 2 deletions lua/orgmode/org/hyperlinks/url.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local fs = require('orgmode.utils.fs')
---@alias OrgUrlPathType 'file' | 'headline' | 'custom-id' | 'id' | 'external-url' | 'plain' | nil
---@alias OrgUrlTargetType 'headline' | 'custom-id' | 'line-number' | 'unknown' | nil

Expand Down Expand Up @@ -116,7 +117,7 @@ function Url:get_file()
if not self:is_file() then
return nil
end
return self.path
return self.realpath or self.path
end

---@return string | nil
Expand Down Expand Up @@ -246,8 +247,12 @@ function Url:_parse_path_type()
return
end

if first_char == '.' and (self.path:sub(1, 3) == '../' or self.path:sub(1, 2) == './') then
if
(first_char == '.' and (self.path:sub(1, 3) == '../' or self.path:sub(1, 2) == './'))
or (first_char == '~' and self.path:sub(2, 2) == '/')
then
self.path_type = 'file'
self.realpath = fs.get_real_path(self.path) or self.path
return
end

Expand Down
4 changes: 4 additions & 0 deletions lua/orgmode/utils/fs.lua
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ function M.get_real_path(filepath)
return false
end
local real = vim.loop.fs_realpath(substituted)
if filepath:sub(-1, -1) == '/' then
-- make sure if filepath gets a trailing slash, the realpath gets one, too.
real = real .. '/'
end
return real or false
end

Expand Down
30 changes: 30 additions & 0 deletions tests/plenary/helpers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,38 @@ local function create_file_instance(lines, filename)
return file
end

---@return table
local function create_agenda_files(filenames, contents)
-- NOTE: content is only 1 line for 1 file
local temp_fname = vim.fn.tempname()
local temp_dir = vim.fn.fnamemodify(temp_fname, ':p:h')
-- clear temp dir
vim.fn.delete(temp_dir .. '/*', 'rf')
local files = {}
local agenda_files = {}
for i, filename in ipairs(filenames) do
local fname = temp_dir .. '/' .. filename
fname = vim.fn.fnamemodify(fname, ':p')
if fname then
local dir = vim.fn.fnamemodify(fname, ':p:h')
vim.fn.mkdir(dir, 'p')
vim.fn.writefile({ contents[i] }, fname)
files[filename] = fname
table.insert(agenda_files, fname)
end
end
local cfg = vim.tbl_extend('force', {
org_agenda_files = agenda_files,
}, {})
local org = orgmode.setup(cfg)
org:init()
return files
end
Comment on lines +38 to +64
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
---@return table
local function create_agenda_files(filenames, contents)
-- NOTE: content is only 1 line for 1 file
local temp_fname = vim.fn.tempname()
local temp_dir = vim.fn.fnamemodify(temp_fname, ':p:h')
-- clear temp dir
vim.fn.delete(temp_dir .. '/*', 'rf')
local files = {}
local agenda_files = {}
for i, filename in ipairs(filenames) do
local fname = temp_dir .. '/' .. filename
fname = vim.fn.fnamemodify(fname, ':p')
if fname then
local dir = vim.fn.fnamemodify(fname, ':p:h')
vim.fn.mkdir(dir, 'p')
vim.fn.writefile({ contents[i] }, fname)
files[filename] = fname
table.insert(agenda_files, fname)
end
end
local cfg = vim.tbl_extend('force', {
org_agenda_files = agenda_files,
}, {})
local org = orgmode.setup(cfg)
org:init()
return files
end
---@param agenda_file {filename: string, content: string[] }[]
---@return table
local function create_agenda_files(fixtures)
-- NOTE: content is only 1 line for 1 file
local temp_fname = vim.fn.tempname()
local temp_dir = vim.fn.fnamemodify(temp_fname, ':p:h')
-- clear temp dir
vim.fn.delete(temp_dir .. '/*', 'rf')
local files = {}
local agenda_files = {}
for fixture in pairs(fixtures) do
local fname = temp_dir .. '/' .. fixture.filename
fname = vim.fn.fnamemodify(fname, ':p')
if fname then
local dir = vim.fn.fnamemodify(fname, ':p:h')
vim.fn.mkdir(dir, 'p')
vim.fn.writefile(fixture.content, fname)
files[filename] = fname
table.insert(agenda_files, fname)
end
end
local cfg = vim.tbl_extend('force', {
org_agenda_files = agenda_files,
}, {})
local org = orgmode.setup(cfg)
org:init()
return files
end

This is, what I imagine as the modified interface of the helper function.


return {
load_file = load_file,
create_file = create_file,
create_file_instance = create_file_instance,
create_agenda_file = create_agenda_file,
create_agenda_files = create_agenda_files,
}
33 changes: 33 additions & 0 deletions tests/plenary/org/autocompletion_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,39 @@ describe('Autocompletion', function()
{ menu = '[Org]', word = 'Title without anchor' },
}, result)
end)

it('should work on relative paths', function()
local files = helpers.create_agenda_files({
'a.org',
'b/c.org',
}, {
'[[./ ',
'[[../ ',
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't want to be a nitpick, but I would prefer to clean each test a bit up to be specific for the case. Makes it easier for future contributions to not be distracted by code, which is actually irrelevant for the test case.

Copy link
Author

Choose a reason for hiding this comment

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

I agree with you.

The case here is mainly about paths, so the content does not really matter. So do you suggest me to add some extra info to the files?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I had a misconception, how the create_agenda_files function works. The position in the file table corresponds with the position in the content table.

So I think, this cannot be improved further with reasonable amount of effort.

Copy link
Author

Choose a reason for hiding this comment

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

Fine, so we just keep it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I also see now, that you contributed this function - I thought it already existed.

I have actually one last improvement: Can you change the interface of the function in that way, that it takes a list of tables, each one for each agenda file. And that each entry in this table is again an associative table with the keys filename and content. While filename is just a string, content would be again a table, each entry a line. This makes your function reusable for more complicated use cases with multiline content for an agenda file. I already have several test cases in mind, where this might be very useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

And sorry for the inconvenience, I use the review functions of Github actually the first time and just found the "Review in workspace" feature, which is actually great. Wished I found it earlier.

})
Comment on lines +344 to +350
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
local files = helpers.create_agenda_files({
'a.org',
'b/c.org',
}, {
'[[./ ',
'[[../ ',
})
local files = helpers.create_agenda_files({
{
filename = 'a.org',
content = {},
},
{
filename = 'b.org',
content = { {'[[../'} }
}

More specific test case A.

helpers.load_file(files['b/c.org'])
vim.fn.cursor({ 1, 6 })

assert.are.same({
{ menu = '[Org]', word = '../a.org' },
{ menu = '[Org]', word = '../b/c.org' },
}, org.completion:omnifunc(0, '../'))
end)
it('should work on relative paths', function()
local files = helpers.create_agenda_files({
'a.org',
'b/c.org',
}, {
'[[./ ',
'[[../ ',
})
Comment on lines +360 to +366
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
local files = helpers.create_agenda_files({
'a.org',
'b/c.org',
}, {
'[[./ ',
'[[../ ',
})
local files = helpers.create_agenda_files({
{
filename = 'a.org',
content = { {'[[./'}},
},
{
filename = 'b.org',
content = {}
}
})

More specific test case B.

helpers.load_file(files['a.org'])
vim.fn.cursor({ 1, 5 })

assert.are.same({
{ menu = '[Org]', word = './a.org' },
{ menu = '[Org]', word = './b/c.org' },
}, org.completion:omnifunc(0, './'))
end)
end)
end)
end)
Loading