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

Make notmuch db location logic match upstream #1495

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kirelagin
Copy link

  • Allow relative database.path in notmuch config.
  • Fallback to $MAILDIR or $HOME/mail.

Fixes #1396.


I am sorry, I have no idea how to write tests in Python, in particular, how to define helper constants in them. I suspect I should have put them not at the toplevel, but in the class, but I don’t know how to do that, please, help.

* Allow relative database.path in notmuch config.
* Fallback to $MAILDIR or $HOME/mail.
@kirelagin
Copy link
Author

Actually, I wish this code was in the bindings or in the C functions that bindings go through.

@pazz
Copy link
Owner

pazz commented Apr 19, 2020

I agree: this is something that the notmuch bindings should expose and we should not re-implement if it already exists as a (c) library function.

I don't have time to look into this at the moment, but perhaps you could write to the notmuch list if this is not already in the bindings and ask why not, or propose a change to the bindings before you come back here to propose a PR that simply uses the bindings? I believe that'd be the cleanest solution.

@kirelagin
Copy link
Author

This problem is that this logic is implemented in notmuch_config_open and the only use of this function is directly in main. As far as I can tell, there is nothing in bindings for working with the configuration, so exposing this logic from the C code will require quite a bit of restructuring. I wouldn’t hope that this is something that will be trivial to do.

@pazz
Copy link
Owner

pazz commented Jun 13, 2020

I've been thinking about this again and it seems to be quite a big patch for a minimal change.
In fact we can assume that the notmuch config does set the path variable, so there is really no need to set an explicit default value.
Secondly, why not sumply replace this line with

indexpath = os.path.abspath(settings.get_notmuch_setting('database', 'path'))

This will replace any relative path by its absolute path.

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.

Support notmuch database paths without leading /
2 participants