joostkremers / ebib

A BibTeX database manager for Emacs.
https://joostkremers.github.io/ebib/
BSD 3-Clause "New" or "Revised" License
272 stars 37 forks source link

Use button functionality to make some fields clickable (now including `crossref`) #265

Closed Hugo-Heagren closed 1 year ago

Hugo-Heagren commented 1 year ago

Replaces #264.

Use Emacs-standard button library to make some text clickable, replacing the previous mechanism. Reasons are given in the commit messages and #264. Make crossref fields clickable, to follow the crossref (similar to opening a file).

joostkremers commented 1 year ago

I get a bunch of compiler warnings trying to compile this PR. Among others, they include a bunch of these:

ebib-utils.el:2397:10: Warning: button-buttonize called with 4 arguments, but
    accepts only 2-3

I'm running an Emacs pretest version (28.1.91), so perhaps this will change in Emacs 29, but for now, button-buttonize takes three arguments, the third one optional. The third argument should be a list, I assume:

        (button-buttonize "www"
                  'ebib--call-browser ;; callback
                  (list str           ;; data (passed to callback)
                        str))         ;; help-echo

I'm also adding a few inline comments.

joostkremers commented 1 year ago

I'm running an Emacs pretest version (28.1.91), so perhaps this will change in Emacs 29, but for now, button-buttonize takes three arguments, the third one optional. The third argument should be a list, I assume:

That doesn't make sense, actually... If the fourth argument is a help echo for the button, it would be weird to pass it to the callback function.

joostkremers commented 1 year ago

Hmm, I now notice that button-buttonize was introduced with Emacs 28. That's a problem, because I try to keep Ebib compatible with the pre-previous Emacs version. (Some distros take long to update...) So currently that's Emacs 26.1, when Emacs 29 is released, it'll be Emacs 27.1.

BTW, just checked the source. On master, button-buttonize is simply called buttonize and takes a fourth argument. button-buttonize is an obsolete alias for this function.

But given that this is on master, the fourth argument is probably not going to appear in Emacs 29, because we're already in the pretest phase for 29.

Hugo-Heagren commented 1 year ago

As far as I can see then:

So I'll just add the properties 'manually' with propertize. This also completely removes the problem of which versions of the function have which arguments.

Hugo-Heagren commented 1 year ago

So I'll just add the properties 'manually' with propertize. This also completely removes the problem of which versions of the function have which arguments.

Done, and pushed the new versions of the relevant commits.

joostkremers commented 1 year ago

Hey, it took a bit of time for me to take a look at this more closely. Looks good, except for one small thing: The notes symbol in the index buffer was also clickable: it was handled by ebib-index-open-at-point. You'll need to give the function ebib-notes-display-note-symbol the same treatment as ebib-display-www-link. Or wait until I do it. :smile:

Hugo-Heagren commented 1 year ago

You'll need to give the function ebib-notes-display-note-symbol the same treatment as ebib-display-www-link

Done!

joostkremers commented 1 year ago

Finally got to take another look and merged this. Thanks!