orgmode icon indicating copy to clipboard operation
orgmode copied to clipboard

feat(links)!: Use refactor links structure

Open kristijanhusak opened this issue 1 year ago • 5 comments

Start using new links structure introduced in https://github.com/nvim-orgmode/orgmode/pull/802.

@chipsenkbeil can you check if using this branch breaks anything in org-roam.nvim? It shouldn't, but it's best to double check. You will need to introduce one change here: https://github.com/chipsenkbeil/org-roam.nvim/blob/192732a8de167e7770b12db31e737239311b1a43/lua/org-roam/setup/plugin.lua?plain=1#L20

I added a hyperlink class in https://github.com/nvim-orgmode/orgmode/blob/71c57cab31b78d2c6907d2961b6c2666ea9383a1/lua/orgmode/org/links/hyperlink.lua?plain=1#L8, and it works basically the same for your usage. Underlining url prop is also a different class, but it has the get_id method that you are using.

For this line you can use Hyperlink.at_cursor function instead of these 3 lines to get current link.

Besides this I think everything should be working as expected, but please give it a thorough test.

@seflue @SlayerOfTheBad could you switch to this branch and see if everything related to hyperlinks works as expected?

Thanks!

kristijanhusak avatar Aug 23 '24 14:08 kristijanhusak

@chipsenkbeil @seflue @SlayerOfTheBad mentioning just in case you didn't receive the notification, since I mentioned you in an edit of a description.

kristijanhusak avatar Aug 23 '24 16:08 kristijanhusak

@kristijanhusak thanks for keeping me in the loop :) I got your first mention, but glad you double-checked.

I can get this change into the plugin in the next day or two. Is this going to mark a new release? Wondering if I should

  1. Mark a new release that locks to a new version of the orgmode plugin so I can change the code to use the update
  2. Or, update my code to try to use the new method and fall back to the old approach if it doesn't exist, thereby supporting both changes. Would still mean that folks using older versions of my plugin would break if they updated to your latest

Leaning towards the former if you're planning to mark this as a new orgmode release 😄

chipsenkbeil avatar Aug 23 '24 16:08 chipsenkbeil

I put out a release 0.3.5 yesterday that does not have any of these link changes. You can pin your current version to this version.

Once i merge this, i can tag another one, but I'd prefer to iron out any bugs before doing that.

Edit: Note that your current code should not break even on this PR since i didn't remove old code, but please double check.

kristijanhusak avatar Aug 23 '24 16:08 kristijanhusak

Edit: Note that your current code should not break even on this PR since i didn't remove old code, but please double check.

Ah, okay! I thought this was a callout to breaking change. If this is not breaking then even better.

I think I'd like to wait towards making the change until after the tag, but I can create a branch of my plugin with the change to verify that it works with the new code. I just won't merge it until I can tag a new release on my side that syncs up with yours.

So TLDR, I'll help verify that things work and the change behaves as expected, but will wait to land until after it rolls out in a tagged orgmode release.

chipsenkbeil avatar Aug 23 '24 16:08 chipsenkbeil

@chipsenkbeil @seflue @SlayerOfTheBad mentioning just in case you didn't receive the notification, since I mentioned you in an edit of a description.

@kristijanhusak Thanks for the reminder. I will check, if I need to adjust something in the next days and update you here.

seflue avatar Aug 23 '24 19:08 seflue

@chipsenkbeil @seflue did you had a chance to test this? If yes, any issues found?

kristijanhusak avatar Sep 16 '24 15:09 kristijanhusak

I'm merging this in. We can solve any issues as we go. Org-roam suggests using 0.3.4 version so we should be ok.

kristijanhusak avatar Sep 18 '24 10:09 kristijanhusak

@kristijanhusak all good to merge since I lock versions.

Haven't set aside time to check. Will make a note to try to do so this week.

chipsenkbeil avatar Sep 18 '24 12:09 chipsenkbeil

@kristijanhusak months later, and while I didn't explicitly check anything I've been on the latest for org-roam.nvim and orgmode without issue, so I think the links are fine.

chipsenkbeil avatar Nov 16 '24 20:11 chipsenkbeil

I can agree from my side. I am using telescope-orgmode on a daily basis, especially to create links and I have no issues with the latest version of nvim-orgmode.

seflue avatar Nov 24 '24 21:11 seflue

Awesome, thanks!

kristijanhusak avatar Nov 25 '24 08:11 kristijanhusak