emacs-citar / citar

Emacs package to quickly find and act on bibliographic references, and edit org, markdown, and latex academic documents.
GNU General Public License v3.0
501 stars 54 forks source link

Make citar-{bibliography,library-paths,notes-paths} safe local vars #800

Closed ashton314 closed 4 months ago

ashton314 commented 1 year ago

It is incredibly handy to customize citar-bibliography as a directory-local variable on a per-project basis. This worked so fabulously with the paper I just finished that I can't see myself not using this workflow again. I think a lot of people would like to use Citar in this way too.

I've added autoload cookies to put the safe local variable predicate on the symbol; the predicate is simply a list of strings. I followed how Denote by Prot was setting the safe local variable predicate on some denote variables.

bdarcus commented 1 year ago

Thanks for this!

Can you please clarify, including in the README, how and why one would use this, rather than just local bibliographies?

Also, while I can just squash the commit(s) and rewrite the commit message, note that we've been using conventional commit guidelines recently. If you're comfortable rewriting history yourself, that would save me the trouble ;-)

ashton314 commented 1 year ago

Conventional Commits are new to me—I've seen them but not used them. Would you consider this a feat?

ashton314 commented 1 year ago

OK @bdarcus I've updated the commit message and added a little blurb in the README—happy to expand if you'd like.

If you're comfortable rewriting history…

Magit makes this dangerously easy. :) I can't imagine using Git now without it. In fact, I opened this fork and PR entirely with Magit+Forge!

While I've got your attention, thank you so much for making this. The other day I was writing up a long blog post, and I was so thrilled that I could use the exact same workflow for adding citations to my blog (using ox-hugo) as I do for writing papers. ❤️ so cool

roshanshariff commented 1 year ago

I'd also like to hear about your workflow @ashton314. For the bibliographies, perhaps it makes more sense to have a separate buffer-local variable, perhaps citar-local-bibliography, which is added to the list of buffer-local bibliographies parsed from Latex, Org, Markdown, etc.

And while I can also see the utility in project-specific notes, I'm curious to learn how you're using per-project library paths.

I'm not opposed to marking these variables safe-local, but first I'd like to understand if there are better ways of accomplishing the purpose you had in mind. For instance, just overriding these variables locally makes it hard to do things like configurably combining local and global paths.

bdarcus commented 1 year ago

Conventional Commits are new to me—I've seen them but not used them. Would you consider this a feat?

Yes.

You'd want the first line of the commit message to be 50 characters max, and lowercase.

Thanks for the README addition, but I'm still not totally clear, nor it seems is @roshanshariff. Could you please respond to him?

ashton314 commented 1 year ago

Hey—sorry for the long wait. Semester started and I got inundated. 😅 Surfacing now!

I used a local value for citar-bibliography for a shared paper to great effect. This is what I had at the bottom of my .tex file:

% Local Variables:
% jinx-local-words: "..."
% citar-bibliography: ("./ref.bib")
% End:

I was collaborating with several other authors—this made it nice for me to scope citar's UI to just the papers that we had in our bibliography. If a ref I wanted was missing from the list, it was a good clue for me to go back to my personal .bib file and add it.

I'll confess I haven't actually used the other two variables: they seemed like good variables to change in tandem with the citar-bibliography file, as I (perhaps mis-)understood it, these paths should be somewhat related to one another.

If going the route of a buffer-local variable, I think it might be nice to have a switch or some kind of narrowing key to restrict citar results to only what is available locally. That way a missing ref in the project-specific .bib file can get caught faster before trying to render the paper.

ashton314 commented 1 year ago

If you'd prefer; I'll narrow the scope of this PR to just updating the citar-bibliography variable and we can discuss the other two separately. Lmk what you'd like me to do.

bdarcus commented 1 year ago

If going the route of a buffer-local variable, I think it might be nice to have a switch or some kind of narrowing key to restrict citar results to only what is available locally.

I think this is related to #695.

If you'd prefer; I'll narrow the scope of this PR to just updating the citar-bibliography variable and we can discuss the other two separately. Lmk what you'd like me to do.

I'm unsure, since safe local variables are new to me.

Let's see if @roshanshariff has further input, though I know he, like us, is busy.

roshanshariff commented 1 year ago

Maybe we can think of the two requested subfeatures separately?

  1. Specifying local bibliographies: we currently support mode-specific ways of doing this, like the \bibliography or \addbibresource macros in Latex (handled by Reftex) or the #+bibliography keyword in Org mode. If these aren't sufficient (are they?) we could add a safe local variable that lets you specify more files, maybe citar-local-bibliography.
  2. Restricting the citar interface to local bibliographies only. A less intrusive alternative would be to add a custom indicator as in #695. If that's not sufficient, we could add a local minor mode (citar-local-only-mode?) that makes citar only show entries from local bibliographies. I'm inclined not to make this a safe local variable because I don't want files to be able to silently alter the behaviour of Citar, which I rely on to access my bibliography throughout Emacs. But, of course, you could always add the variable to safe-local-variable-values if you want to allow this, and Emacs will prompt you to do so if you open a file that tries to set it.
bdarcus commented 1 year ago
  1. If these aren't sufficient (are they?) ...

And if they aren't, why not?

The advantage of how we do things now is what local files tex, org etc see is the same as what citar sees.

aikrahguzar commented 11 months ago

The advantage of how we do things now is what local files tex, org etc see is the same as what citar sees.

I think this is a big advantage and reading the discussion, it seems like the same use case can be met by just unsetting the citar-bibliography. Citar will use just the local bibliographies in that case. So, in my mind the only local value for citar-bibliography ever needed should be nil. Is there any use case not covered by that?

ashton314 commented 6 months ago

(Hey, sorry again for the super long delay. I've been slammed this semester.)

I didn't realize that citar would read what tex and org see—that's perfect.

In that case, I am in favor of making nil the only baked-in safe buffer-local value for citar-bibliography. I will update my PR to reflect this.

ashton314 commented 6 months ago

@bdarcus I think this is ready now! Thanks all for your patience.

roshanshariff commented 6 months ago

@ashton314,

Citar uses local bibliographies in addition to those set in citar-bibliography. It combines the entries from all local and global bibliographies. In particular, you don't need to set the global bibliography to nil to use local bibliographies.

So if your collaborators don't use Citar themselves, they will have no global bibliographies and only local ones will be used. If they do have global bibliographies set up, I'm not sure they would expect Citar to automatically, without warning, ignore them when inside a particular project.

I see that you prefer the global bibliographies to be suppressed for certain projects, but since allowing this is a personal preference, it naturally belongs in your own private configuration?

bdarcus commented 4 months ago

Per Roshan's explanation, and in https://github.com/emacs-citar/citar/pull/831#issuecomment-2105980044, I am closing this.

Thanks for the PR, @ashton314, but we'll need to find another way (which Roshan suggests in the above linked comment).