awth13 / org-appear

Toggle visibility of hidden Org mode element parts upon entering and leaving an element
MIT License
375 stars 19 forks source link

Toggle just brackets around links #44

Closed jun0 closed 2 years ago

jun0 commented 2 years ago

Hey! Nice work here! I really like this package. It solves a major gripe with vanilla org-mode in a really elegant way :) I was wondering if you can adopt this small enhancement, though.

Showing everything inside a link can be disorienting, as URLs tend to be long, and showing them shifts all subsequent text by many characters. The URL is also usually not interesting: it's just a technical detail of a link which is rarely inspected or updated.

This PR allows org-appear-autolinks to be set to 'just-brackets, which instructs org-appear to show just a pair of brackets around a link under point, and no more. This way, I get solid visual feedback on whether I'm typing inside or outside the link text, without seeing the link expand into a huge blob. If I ever need to view/edit the URL, I can still use org-insert-link.

awth13 commented 2 years ago

Hi and thank you for the PR, @jun0! I am sorry for not responding earlier.

I tried it out and it looks very good indeed. I do, however, have some comments about the code and formatting. Would you mind addressing these within the PR?

jun0 commented 2 years ago

Hi, glad to know you find it useful too. Sure, I can adjust things, no problem. What could I do to make it better?

awth13 commented 2 years ago

Thank you for implementing the changes!

While your approach of defining the type as '(choice boolean (const just-brackets)) works, it is a little confusing for the end user in the customisation buffer. The user can toggle the boolean or choose the special just-brackets value. If I didn't know what's going on here, I would maybe assume that if I toggled it off, org-appear would ignore links regardless of just-brackets.

I suggest the following definition instead. It gives the user a clear choice of 3 options. Ideally, we would use another symbol instead of t for toggling full links but this would break existing user installs so I decided against it.

:type '(choice (const :tag "Ignore links" nil)
           (const :tag "Toggle full link" t)
           (const :tag "Toggle brackets" just-brackets))

Could you please implement this final change?

jun0 commented 2 years ago

Good point! I've implemented the change.

awth13 commented 2 years ago

Thank you for your contribution! Merged.