Skip to content

Extend API to get links to headline and org file #768

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jul 1, 2024
Merged

Extend API to get links to headline and org file #768

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f255ac5
feat(api): Extend API to get links
seflue Jun 30, 2024
5a88955
chore: Remove stale require
seflue Jun 30, 2024
1344eb1
chore: tiny refactoring to align similar methods
seflue Jun 30, 2024
23f0c98
refactor: make get_link member functions
seflue Jul 1, 2024
cb621f0
fix: handle unloaded buffers
seflue Jul 1, 2024
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
61 changes: 60 additions & 1 deletion lua/orgmode/api/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
local OrgFile = require('orgmode.api.file')
local OrgHeadline = require('orgmode.api.headline')
local Hyperlinks = require('orgmode.org.hyperlinks')
local Link = require('orgmode.org.hyperlinks.link')
local orgmode = require('orgmode')

---@class OrgApiRefileOpts
Expand Down Expand Up @@ -101,7 +100,67 @@ function OrgApi.refile(opts)
return true
end

--- Get a link destination as string
---
--- Depending if org_id_link_to_org_use_id is set the format is
---
--- id:<uuid>::*title and the id is created if not existing
--- or
--- file:<filepath>::*title
---
--- The result is meant to be used as link_location for OrgApi.insert_link.
--- @param headline OrgApiHeadline
--- @return string
function OrgApi.get_link_to_headline(headline)
local filename = headline.file.filename
local bufnr = vim.fn.bufnr(filename)

if bufnr == -1 then
Copy link
Member

@kristijanhusak kristijanhusak Jun 30, 2024

Choose a reason for hiding this comment

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

Why do we need to do this if there is no buffer?

Copy link
Contributor Author

@seflue seflue Jul 1, 2024

Choose a reason for hiding this comment

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

Jep, actually this is the crucial part (and took me some time to figure out, reading through the agenda code).

The file we want to target a link to, is very likely not open. If we use org_id_link_to_org_use_id = true, there might not be an ID yet. But creating this id needs an edit of the file, which is done by the id_get_or_create methods under the hood. And this needs to open the file automatically in the background and close it again, if it is not already open.

Let me clarify the motivation behind this API extension:
When I use telescope-orgmode to create links, I can access the whole index of files build up by orgmode on startup. This allows for very convenient creation of links and cross-references using fuzzy finding. It is not very convenient, if I would have to open the files first, before being able to set the link.
The manual workflow for ID links works actually that way, I have to open the target file, press org-store-link (which also creates the id), navigate to my file, where the link originates and press org-insert-link. My telescope plugin makes this workflow much smoother, because I can just filter for the target headline without open the file, press enter and, boom, I have the link and the ID.

Copy link
Member

@kristijanhusak kristijanhusak Jul 1, 2024

Choose a reason for hiding this comment

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

Ok, sounds good.
Is it worth moving these methods to OrgApiFile and OrgApiHeadline instead of passing them down from the top level?

Copy link
Contributor Author

@seflue seflue Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think, we should keep it as it is for know. I expect to discover more requirements while I am implementing more features and playing around with what we have. If it doesn't bother you, I would adapt when needed and we have better knowledge.

Copy link
Member

@kristijanhusak kristijanhusak Jul 1, 2024

Choose a reason for hiding this comment

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

I just figured out I didn't phrase my question correctly.

What I wanted to ask is:
Would it work for you to move these to api/file and api/headline? All of the file or headline specific api methods live in those file, so I'd prefer to keep it consistent.

If it doesn't work for you, can you clarify why?

Copy link
Contributor Author

@seflue seflue Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

All three methods are used for link creation and are meant to be used together. I might need to understand better, what your primary criterion is for where to put a method and also what the main purpose of the top-level API is.
I don't actually have a big problem of making these methods member functions of OrgApiFile and OrgApiHeadline.

Copy link
Member

@kristijanhusak kristijanhusak Jul 1, 2024

Choose a reason for hiding this comment

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

I might need to understand better, what your primary criterion is for where to put a method

If a function is accepting an instance of something, I always prefer to have that function on the instance itself. Of course that's not always possible, but when it is, I prefer to take that path.
For example, if we would follow this way for example for get_property, we would have something like this:

orgmode.api.get_property(headline, 'id')

But this is much cleaner IMO:

headline:get_property('id')

Copy link
Contributor Author

@seflue seflue Jul 1, 2024

Choose a reason for hiding this comment

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

Yes, was already convienced, when I realized, that the argument of the original functions is actually the self argument in orgmode.api.headline and orgmode.api.file. . 😄
Now that I refactored the code I am even more convienced, because it cleaned also things up on the client side.

What do you think, are we ready to merge?

-- do remote edit
return orgmode.files
:update_file(filename, function(_)
return Hyperlinks.get_link_to_headline(headline._section)
end)
:wait()
end

return Hyperlinks.get_link_to_headline(headline._section)
end

--- Get a link destination as string
---
--- Depending if org_id_link_to_org_use_id is set the format is
---
--- id:<uuid>::*title and the id is created if not existing
--- or
--- file:<filepath>::*title
---
--- The result is meant to be used as link_location for OrgApi.insert_link.
--- @param file OrgApiFile
--- @return string
function OrgApi.get_link_to_file(file)
local filename = file.filename
local bufnr = vim.fn.bufnr(filename)

if bufnr == -1 then
-- do remote edit
return orgmode.files
:update_file(filename, function(_file)
return Hyperlinks.get_link_to_file(_file)
end)
:wait()
end

return Hyperlinks.get_link_to_file(file._file)
end

--- Insert a link to a given location at the current cursor position
---
--- The expected format is
--- <protocol>:<location>::<in_file_location>
---
--- If <in_file_location> is *<headline>, <headline> is used as prefilled description for the link.
--- If <protocol> is id, this format can also be used to pass a prefilled description.
--- @param link_location string
--- @return boolean
function OrgApi.insert_link(link_location)
Expand Down
26 changes: 26 additions & 0 deletions lua/orgmode/files/file.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,7 @@ end

memoize('get_category')
--- Get the category name for this file
--- If no category is set, the filename without extension is returned
--- @return string
function OrgFile:get_category()
local category = self:_get_directive('category')
Expand All @@ -653,6 +654,19 @@ function OrgFile:get_category()
return vim.fn.fnamemodify(self.filename, ':t:r') or ''
end

memoize('get_title')
--- Get the title for this file
--- If no title is set, the filename without extension is returned
--- @return string
function OrgFile:get_title()
local title = self:_get_directive('title')
if title then
return title
end

return vim.fn.fnamemodify(self.filename, ':t:r') or ''
end

memoize('get_opened_unfinished_headlines')
---@return OrgHeadline[]
function OrgFile:get_opened_unfinished_headlines()
Expand Down Expand Up @@ -703,6 +717,18 @@ function OrgFile:get_directive(directive_name)
return self:_get_directive(directive_name)
end

--- Get headline id or create a new one if it doesn't exist
--- @return string
function OrgFile:id_get_or_create()
local id = self:get_property('id')
if id then
return id
end
local org_id = require('orgmode.org.id').new()
self:set_property('ID', org_id)
return org_id
end

---@private
---@return string | nil
function OrgFile:_get_directive(directive_name)
Expand Down
26 changes: 21 additions & 5 deletions lua/orgmode/org/hyperlinks/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,32 @@ end
---@param headline OrgHeadline
---@param path? string
function Hyperlinks.get_link_to_headline(headline, path)
path = path or utils.current_file_path()
local title = headline:get_title()
local id

if config.org_id_link_to_org_use_id then
id = headline:id_get_or_create()
local id = headline:id_get_or_create()
if id then
return ('id:%s::*%s'):format(id, title)
end
end

if config.org_id_link_to_org_use_id and id then
return ('id:%s::*%s'):format(id, title)
path = path or utils.current_file_path()
return ('file:%s::*%s'):format(path, title)
end

---@param file OrgFile
---@param path? string
function Hyperlinks.get_link_to_file(file, path)
Copy link
Member

@kristijanhusak kristijanhusak Jul 1, 2024

Choose a reason for hiding this comment

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

Is a file path with the title valid format for a file link?

From the examples here https://orgmode.org/guide/Hyperlinks.html I don't see any of those.

I think for link to file it is enough to either be an id, or just file:/path/to/file.org if there is no id.

Copy link
Contributor Author

@seflue seflue Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It is not, and it's also not treated such. I am piggy-backing here on the "title" format to sneak a description into insert_link to get the form prefilled.
This is something, which actually needs to get cleaned up a bit more properly:
get_link should actually return a table or even better an api equivalent of Url. And insert_link should take that. But this would need some more decent refactoring effort which touches a lot of places (think also of store_link) so I would prefer to defer this to a separate PR.

Right now I am quite happy, how Telescope-orgmode works although the API is not perfect yet.

Copy link
Member

@kristijanhusak kristijanhusak Jul 1, 2024

Choose a reason for hiding this comment

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

Ok, we can leave it for the time being, but later once we do some refactoring, I'd prefer to remove it.
It can also cause side effects in cases where #+TITLE or the file name is the same as some other headline value. In that case, the link would jump to the headline.

local title = file:get_title()

if config.org_id_link_to_org_use_id then
local id = file:id_get_or_create()
if id then
return ('id:%s::*%s'):format(id, title)
end
end

path = path or file.filename
return ('file:%s::*%s'):format(path, title)
end

Expand Down