Re: [PATCH] config: Expand ~ to $HOME

Subject: Re: [PATCH] config: Expand ~ to $HOME

Date: Mon, 09 May 2016 10:54:21 +0300

To: Bijan Chokoufe, notmuch@notmuchmail.org

Cc:

From: Tomi Ollila


On Sun, May 08 2016, Bijan Chokoufe <bijan@chokoufe.com> wrote:

> Hi Tomi,
>
> Thanks for your detailled review. Please see questions below.
>
> Cheers,
> Bijan
>
> Tomi Ollila <tomi.ollila@iki.fi> schrieb am So., 8. Mai 2016 um 18:47 Uhr:
>
>> On Sun, May 08 2016, Bijan Chokoufe Nejad <bijan@chokoufe.com> wrote:
>>
>> > Very useful in case you want to keep your .notmuch-config synchronized
>> across
>> > machines where you have different user names.
>>
>> Thank you for your interest in improving notmuch!
>>
>> There are a few things that needs to be sorted out for this feature to be
>> good:
>>
>> This implementation does not handle ~user/ prefix: i.e. home directory of
>> 'user' (maybe this should not, but it should handle the case).
>
> I don't get it. Is '~user" an alternative to '~'?

~user is ~ in case you're 'user' -- except that now that I think of it
~user could read home directory from /etc/passwd and not using $HOME.
If you're 'eve', then ~alice should definitely be different than ~

>
>> Whether or not ~user is handled, it should check that slash (/) follows...
>>
>>
> 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`.

your code checked that path[0] == '~', but nothing else, e.g.

~123randomstuff/... would expand as /home/user23randomstuff/... 

... and you have good point path being set as single '~' !

>> IIRC there is some ready-made implementations of the above -- but if not,
>> one option is to check how (expand-file-name) works in emacs for reference.
>>
>>
> Well there is wordexp (http://linux.die.net/man/3/wordexp) but I wasn't
> sure if I should use it. The getenv just seemed simpler but maybe it is
> necessary.

For the time being we could simply do checking that path[0] == '~' and
path[1] == '/' and then do expansion based on getenv ("HOME") -- and
comment that ~any_user is just not supported there.

>From testing point of view doing one positive test which ensures that when
HOME is a string pointing to valid directory (and does not end with
trailing slash as it usually is) and path in configuration starts with '~/'
(and does not have multiple slashes following) works as expected...

(as a special case, from softare functionality point I don't see problem
setting path=~/ but from usability point that might not be the best -- but
some users may have peculiar preferences... ;D)

Tomi

Thread: