Extend API to get links to headline and org file #768
Conversation
Considers id links, if org-id-link-to-org-use-id is set. Creates ids on file and headline level if needed.
Adapt the style of get_link_to_headline to new get_link_to_file - avoid empty declaration of `id` - move initialization of `path` close to it's usage
lua/orgmode/api/init.lua
Outdated
| local filename = headline.file.filename | ||
| local bufnr = vim.fn.bufnr(filename) | ||
|
|
||
| if bufnr == -1 then |
There was a problem hiding this comment.
Why do we need to do this if there is no buffer?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ok, sounds good.
Is it worth moving these methods to OrgApiFile and OrgApiHeadline instead of passing them down from the top level?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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')There was a problem hiding this comment.
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?
Simplify the code by providing member functions on org.api.file and org.api.headline.
seflue
force-pushed
the
api_extension_get_link
branch
from
d4aeb6b
to
23f0c98
Compare
|
|
||
| ---@param file OrgFile | ||
| ---@param path? string | ||
| function Hyperlinks.get_link_to_file(file, path) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
In Telescope-orgmode I want to support id-based links when
org_id_link_to_org_use_idis set. I already published the code to a draft PR #20.