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

fix(links): insert id link properly #742

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

seflue
Copy link
Contributor

@seflue seflue commented Jun 2, 2024

When inserting a stored id link, it is now properly split into link and description.

For this to work with the current implementation we need actually to insert id and headline title both into the link input. It get's split and correctly formatted afterwards.

@@ -181,10 +181,10 @@ function Hyperlinks.get_link_to_headline(headline, path)
id = headline:id_get_or_create()
end

if not config.org_id_link_to_org_use_id or not id then
return ('file:%s::*%s'):format(path, title)
if config.org_id_link_to_org_use_id and id then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just swapped the logic for easier readability (no double negation).

if not config.org_id_link_to_org_use_id or not id then
return ('file:%s::*%s'):format(path, title)
if config.org_id_link_to_org_use_id and id then
return ('id:%s::*%s'):format(id, title)
Copy link
Contributor Author

@seflue seflue Jun 2, 2024

Choose a reason for hiding this comment

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

Using the same url syntax as in file links enables the Url class to parse the parts correctly the existing parsing logic.

@@ -216,9 +216,10 @@ end
function Hyperlinks.insert_link(link_location)
local selected_link = Link:new(link_location)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The full url including the description a.k.a. headline title is inserted by the user via autocompletion. Link construction parses it and we can get the parts needed parts easily from it to assemble the actual link.

When inserting a stored id link, it is now properly split into link and
description.

For this to work with the current implementation we need actually to
insert id and headline title both into the link input. It get's split
and correctly formatted afterwards.
@seflue seflue force-pushed the fix_insert_link branch from 44cd4f6 to 490331d Compare June 2, 2024 21:36
@seflue
Copy link
Contributor Author

seflue commented Jun 3, 2024

@kristijanhusak I need a reviewer to merge. 😅

@kristijanhusak
Copy link
Member

@chipsenkbeil can you double check if this can cause some issues with your org-roam? I think it shouldn't but I want to be on the safe side.

@chipsenkbeil
Copy link
Contributor

@kristijanhusak nothing stands out as an issue to me. @seflue is using the org roam plugin - now a collaborator on the project - so I'm assuming this stems from an issue encountered while using org roam.

@seflue
Copy link
Contributor Author

seflue commented Jun 5, 2024

I actually only got aware about the functionality due to my exploration into orgroam. Because I still have a large amount of notes not in orgroam, I use it to connect the "old world" and the "new world" in a filepath-agnostic manner.
And immediately discovered, that "insert_link" is generating broken links. Because org-roam generates it's links by itself, the both plugins do not interfere.
I have actually a hard time to imagine, how the old code was supposed to work correctly. With this fix it works smoothly - as I said, I use both plugins together on a daily basis.

@kristijanhusak kristijanhusak merged commit 8e319bf into nvim-orgmode:master Jun 6, 2024
6 checks passed
@seflue seflue deleted the fix_insert_link branch June 10, 2024 19:38
SlayerOfTheBad pushed a commit to SlayerOfTheBad/orgmode that referenced this pull request Aug 16, 2024
When inserting a stored id link, it is now properly split into link and
description.

For this to work with the current implementation we need actually to
insert id and headline title both into the link input. It get's split
and correctly formatted afterwards.

Co-authored-by: Sebastian Flügge <seflue@users.noreply.github.com>
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.

3 participants