Skip to content

Conversation

@anonymousgrasshopper
Copy link
Contributor

See #24.
notmuch_database_open is deprecated since notmuch version 0.32
Using vim.system({ "notmuch", "--version" }) gives access to the installed notmuch version. We can then check that it is greater than or equal to 0.32 to ensure backwards compatibility with older notmuch versions is preserved.

I have only tested this on notmuch 0.39; installing a pre-0.32 version to check it then uses the old api would be a good idea.

closes #24

notmuch_database_open is deprecated since notmuch version 0.32
Version checks ensure backwards compatibility with older notmuch
versions is preserved.
@yousefakbar
Copy link
Owner

Thank you @anonymousgrasshopper for this, it's a solid start.

I left some review comments to run the version check only once and cache it, and a few more nitpicks here and there.

@anonymousgrasshopper
Copy link
Contributor Author

We can perhaps add a config option to do that, so that we can just use the value in the user's config if it exists and else detect it and cache it in the config. This way if someone uses an old notmuch version and doesn't want to/can't update, and are tired of seeing the using notmuch pre-0.32 message each time, then they can write the version in their config to skip the check and hence the message.

@yousefakbar
Copy link
Owner

We can perhaps add a config option to do that, so that we can just use the value in the user's config if it exists and else detect it and cache it in the config.

Do you mean that the users should keep their notmuch version in the config as a configurable option? While I see how that could be useful, I don't feel this is a good idea. The users should be able to get things up and running with minimal configuration.

I don't mind the using notmuch pre-0.32 message showing (or enhanced for more clarity with something like Using notmuch version <0.32 DEPRECATED API. Please consider upgrading), the point is it should only show up once and cached, not run every time the user tags, opens a message, etc. (especially not the notmuch --version system call on every operation).

That way the message will get across to the user, but not too aggressively where it gets tedious.

@yousefakbar
Copy link
Owner

@anonymousgrasshopper BTW, I pushed to feat/migrate-ffi-bindings with your commits plus the performance fix on top to run and cache the notmuch --version once only.

Let me know if you're fine with this to be merged if you want to test it first.

Previously checked notmuch version on every database open. Now detect
once at module initialization and cache the result, reducing subprocess
spawns to only 1 per session
@anonymousgrasshopper
Copy link
Contributor Author

You checked for version 0.40 instead of version 0.32... I don't think 0.40 even exists yet ! I corrected this tiny mistake, should be good to go now.
I agree the user shouldn't have to specify their version number in the config, my point was that getting the warning message each time the lua module is loaded can get kind of tiresome if for some reason they couldn't upgrade notmuch, so adding a config option for saying "I use notmuch <0.32 and I'm aware of that, please stop bothering me with your warning" would make sense. Though adding that isn't exactly the scope of this PR, we could do it later if the need ever arises.

@yousefakbar
Copy link
Owner

You checked for version 0.40 instead of version 0.32... I don't think 0.40 even exists yet ! I corrected this tiny mistake, should be good to go now.

Oops my bad !! 🫣 I left that by accident when I was testing. I'll remove that when I merge.

my point was that getting the warning message each time the lua module is loaded can get kind of tiresome if for some reason they couldn't upgrade notmuch

When require is run, nvim only actually runs the detect_notmuch_version() and subsequent warning message once. Nvim then stores the module in its state so when you require a second time and so on, it uses a cached version hence it doesn't run the detection again nor the warning mesage!

@yousefakbar
Copy link
Owner

Merged to main and closing. Thanks for this!

@anonymousgrasshopper
Copy link
Contributor Author

When require is run, nvim only actually runs the detect_notmuch_version() and subsequent warning message once. Nvim then stores the module in its state so when you require a second time and so on, it uses a cached version hence it doesn't run the detection again nor the warning mesage!

I meant if I open neovim, run the :Notmuch command, then close neovim, and do that again, the warning message will show up twice in total. What you said only applies when requiring a module several times from the same editor instance.

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

Successfully merging this pull request may close these issues.

Migrate FFI bindings from deprecated notmuch_database_open to notmuch_database_open_with_config

2 participants