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

expand proactive reloading to notes/pdf dirs #369

Closed
bdarcus opened this issue Apr 5, 2021 · 19 comments
Closed

expand proactive reloading to notes/pdf dirs #369

bdarcus opened this issue Apr 5, 2021 · 19 comments

Comments

@bdarcus
Copy link

bdarcus commented Apr 5, 2021

In 2018, bibtex-completion added the feature "Reload bibliography proactively when bib files are changed."

The use case is when you're moving back and forth between, say, note taking on sources, and writing document that integrate those sources. With the expansion in last years of note support, and existing pdf integration, this seems like a natural extension.

Is there any reason not to extend this to PDFs and notes?

As in, if I add a PDF or note to my respective configured directories, bibtex-completion will update the candidates.

Something like this?

https://gist.github.com/bdarcus/6fa2918bfe5f4e89af44e2b0ceb0e550

cc @myshevchuk

Originally posted by @bdarcus in #242 (comment)

@tmalsburg
Copy link
Owner

[Copied from #242 discussion]: Good idea. The only downside that I see is that it could cause more reloads than users might like, especially users with very large bibliographies. For instance, say you have all notes in one file and you edit this file frequently and accordingly save it frequently, then Emacs would block every time you save that file and with large bibs like crypto.bib it would block for (I think) more than 30s. This could be addressed by allowing users to configure autoreloading on a fine-grained level but this would be complicated, potentially error prone, and perhaps not worth it. Not sure.

@bdarcus
Copy link
Author

bdarcus commented Apr 8, 2021

This could be addressed by allowing users to configure autoreloading on a fine-grained level but this would be complicated, potentially error prone, and perhaps not worth it. Not sure.

Yeah, I can see the issue with some setups.

I suppose a simple middle point would be to have a defcustom that allowed a user to turn it on or off?

If on, would just use the pdf and notes paths, if they are set.

@tmalsburg
Copy link
Owner

tmalsburg commented Apr 9, 2021

Some other scenarios that are potentially tricky:

  1. If a user also keeps other files in the directories that contain notes and PDFs, any change in those files will also trigger a expensive reloads which block Emacs.
  2. If a user keeps PDFs in many different directories (referenced via the BibTeX file field), we'd have to watch all those directories. And any change in these directories, related or not, will trigger a reload.
  3. In the previous case, we don't even know, a priori, which directories we need to watch.

I'm not saying that we shouldn't expand proactive reloading, but there are many different scenarios that we need to consider carefully before implementing it.

@bdarcus
Copy link
Author

bdarcus commented Apr 9, 2021

Yeah, good points.

Hmm ...

@tmalsburg
Copy link
Owner

There may be a good solution, it's just not immediately clear to me what it is.

@bdarcus
Copy link
Author

bdarcus commented Apr 9, 2021 via email

@tmalsburg
Copy link
Owner

You can already customize this without a hook. Just set up the file watches in your config and let them reload the bibliography when something changes on disk. For one specific setup (e.g. yours) this may actually be doable with a small handful of lines of code. The difficult part is to come up with something that works for all users and all configurations. When I implemented the current version of proactive loading, which is very basic, there were already users whose experience was degraded by this addition. It's important to keep in mind that this package has quite a number of users who all have their own peculiar setups and preferences. There's also other software that depends on bibtex-completion and it's current behavior (org-roam-bibtex, org-ref) and changes like this can easyly break things for them. Not saying that this necessarily would break something but it's my experience that it's surprisingly difficult to anticipate due to all the accumulated complexity.

@bdarcus
Copy link
Author

bdarcus commented Apr 9, 2021

You can already customize this without a hook. Just set up the file watches in your config and let them reload the bibliography when something changes on disk.

In that case, the simplest and most general solution is to not implement this, but put an example somewhere (docs, wiki, etc.).

Not saying that this necessarily would break something but it's my experience that it's surprisingly difficult to anticipate due to all the accumulated complexity.

Yeah; I guess it's not surprising given how many disparate packages people are often trying to get working together.

@tmalsburg
Copy link
Owner

In that case, the simplest and most general solution is to not implement this, but put an example somewhere (docs, wiki, etc.).

Yes, that's a good idea, at least until we have a more general and flexible solution. I will probably use this as well for PDFs since it's easy in my own setup. I use one directory for all PDFs, and only for PDFs. For notes I use a directory that's also my Zettelkasten and which contains a lot of other stuff. So there it's less clear how to do this without triggering many unnecessary reloads.

Yeah; I guess it's not surprising given how many disparate packages people are often trying to get working together.

To be honest, it's a bit of a miracle that Emacs works at all :)

@tmalsburg
Copy link
Owner

tmalsburg commented Apr 9, 2021

I think something like this (not tested) should do the trick for PDFs:

(file-notify-add-watch bibtex-completion-library-path
                       '(change)
                       (lambda (event) (bibtex-completion-candidates)))

For notes you can use the same approach but with bibtex-completion-notes-path. Let me know how it goes.

@bdarcus
Copy link
Author

bdarcus commented Apr 9, 2021

Thanks!

I added it to my (now pretty bare) wiki here:

https://github.com/bdarcus/bibtex-actions/wiki/Configuration#expand-proactive-loading-of-library-to-pdfs-and-notes

PS - closing for now, but will report back once I test.

@bdarcus bdarcus closed this as completed Apr 9, 2021
@tmalsburg tmalsburg reopened this Apr 9, 2021
@tmalsburg
Copy link
Owner

Let's keep this issue open until we have added a working solution to the documentation. Otherwise it will fall through the cracks.

@bdarcus
Copy link
Author

bdarcus commented Apr 9, 2021

Sure.

I seem to have something wrong with my emacs setup ATM, so I'll have to fix that first.

@bdarcus
Copy link
Author

bdarcus commented Apr 9, 2021

I think, aside from the typo, this is right.

But I can't confirm it ATM, I think because of idiosyncrasies in my implementation, which may take me a bit of time to sort out.

@bdarcus
Copy link
Author

bdarcus commented Apr 10, 2021

Two things:

First, to avoid the confusion below, I tested this with helm-bibtex.

Specifically, I:

  1. opened library, and selected "edit notes" for an item without a note
  2. new org buffer opens, and it appears to reload (because the "Done (re) loading" message)

But when I return to the library view, the new state doesn't show.

Not sure why not.

EDIT: deleting non-relevant details.

@bdarcus
Copy link
Author

bdarcus commented Apr 11, 2021

Update: I had a user request I add a cache, and so have done that. He has confirmed the README instructions (adapted from your example) work.

So I think the example should be good to go for your docs.

@tmalsburg
Copy link
Owner

Thanks for the update. I've added a section in the README, see here.

@bdarcus
Copy link
Author

bdarcus commented Apr 12, 2021

I learned something you might appreciate.

If you add an optional parameter, that you don't use, to bibtex-completion-candidates, you can forego the lambda on the add-watch setup.

Explained here:

oantolin/embark#176 (comment)

@tmalsburg
Copy link
Owner

Hm, this would indeed simplifies the setup of the watch but other users of bibtex-completion-candidates will find the seemingly useless extra argument confusing. Better to keep the interface clean the various functions orthogonal (to the extend possible).

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

No branches or pull requests

2 participants