<div dir="ltr">Hi Tomi,<div><br></div><div>Thanks for your detailled review. Please see questions below.</div><div><br></div><div>Cheers,</div><div>Bijan<br><br><div class="gmail_quote"><div dir="ltr">Tomi Ollila <<a href="mailto:tomi.ollila@iki.fi">tomi.ollila@iki.fi</a>> schrieb am So., 8. Mai 2016 um 18:47 Uhr:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sun, May 08 2016, Bijan Chokoufe Nejad <<a href="mailto:bijan@chokoufe.com" target="_blank">bijan@chokoufe.com</a>> wrote:<br>
<br>
> Very useful in case you want to keep your .notmuch-config synchronized across<br>
> machines where you have different user names.<br>
<br>
Thank you for your interest in improving notmuch!<br>
<br>
There are a few things that needs to be sorted out for this feature to be<br>
good:<br>
<br>
This implementation does not handle ~user/ prefix: i.e. home directory of<br>
'user' (maybe this should not, but it should handle the case).<span style="line-height:1.5"> </span><br></blockquote><div> </div><div>I don't get it. Is '~user" an alternative to '~'?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Whether or not ~user is handled, it should check that slash (/) follows...<br>
<br></blockquote><div><br></div><div>So I guess you aim at the case where someone sets `path=~`? On the other hand why is this checking not necessary in the "normal" case where no expanding of `~` is done? Or is it maybe already handled in `lib/database.cc`. Just to be clear I tested that it works currently with `path=~/.mail`.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
IIRC there is some ready-made implementations of the above -- but if not,<br>
one option is to check how (expand-file-name) works in emacs for reference.<br>
<br></blockquote><div><br></div><div>Well there is wordexp (<a href="http://linux.die.net/man/3/wordexp">http://linux.die.net/man/3/wordexp</a>) but I wasn't sure if I should use it. The getenv just seemed simpler but maybe it is necessary.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Something more inline:<br>
<br>
<br>
> ---<br>
> Not sure this is completely plattform independent.<br>
<br>
The implementation looked like it is platform independent -- at least on<br>
plattforms we care about...<br>
<br>
> I also don't know how to implement a unit test for this.<br>
<br>
I know... and I can do that if we get 1) decide that this feature will be<br>
supported and 2) decide how this feature should work and 3) someone(tm)<br>
does proper implementation ;)<br>
<br>
Tomi<br>
<br>
PS: still more to follow below.<br>
<br>
<br>
> ---<br>
> notmuch-config.c | 11 ++++++++++-<br>
> 1 file changed, 10 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/notmuch-config.c b/notmuch-config.c<br>
> index d252bb2..c9f26ef 100644<br>
> --- a/notmuch-config.c<br>
> +++ b/notmuch-config.c<br>
> @@ -605,7 +605,16 @@ _config_set_list (notmuch_config_t *config,<br>
> const char *<br>
> notmuch_config_get_database_path (notmuch_config_t *config)<br>
> {<br>
> - return _config_get (config, &config->database_path, "database", "path");<br>
> + const char* path = _config_get (config, &config->database_path, "database", "path");<br>
> + if (path != NULL && path[0] == '~') {<br>
> + char *home_path = getenv("HOME");<br>
> + char *shortened_path = malloc( sizeof(char) * ( strlen (path) - 1 ) );<br>
> + strncpy(shortened_path, path + 2, strlen(path));<br>
> + return talloc_asprintf (NULL, "%s/%s", home_path, shortened_path);<br>
<br>
In the implementation above matching free() for malloc() is not done -- but<br>
actually the malloc is unnecessary -- path + 2 could have been used there<br>
(after one checked that path[1] is '/' (provided that ~/ were the only<br>
thing we supported...))<br>
<br>
In strncpy() above length arg is strlen(path) but there is 1 byte less<br>
allocated in shortened_path -- if src arg in the above strncpy() were<br>
something else it could overwrite the allocated space by 2 bytes.<br>
<br></blockquote><div><br></div><div>Yes, sorry. My C is a little rusty ;). I was happy when it worked. But when we agree that wordexp is the way to go, this can be removed anyhow.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
> + }<br>
> + else {<br>
> + return path;<br>
> + }<br>
> }<br>
><br>
> void<br>
> --<br>
> 1.9.1<br>
><br>
> _______________________________________________<br>
> notmuch mailing list<br>
> <a href="mailto:notmuch@notmuchmail.org" target="_blank">notmuch@notmuchmail.org</a><br>
> <a href="https://notmuchmail.org/mailman/listinfo/notmuch" rel="noreferrer" target="_blank">https://notmuchmail.org/mailman/listinfo/notmuch</a><br>
</blockquote></div></div></div>