Re: [PATCH] notmuch-mutt: check that the search cache Maildir is not a real Maildir WAS: Re: Data loss

Subject: Re: [PATCH] notmuch-mutt: check that the search cache Maildir is not a real Maildir WAS: Re: Data loss

Date: Sun, 28 May 2023 10:27:38 +0800

To: notmuch

Cc:

From: Paul Wise


On Sat, 2023-05-27 at 21:10 -0300, David Bremner wrote:

> If possible, please use git-send-email for future series. 

Will do, I forgot about the --cover-letter option.

> This seems to have introduced a warning, and I think there's a bug.
> 
> % notmuch mutt search tag:inbox 
> Argument "new" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt line 70.
> Argument "cur" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt line 70.
> Argument "tmp" isn't numeric in array or hash lookup at /usr/bin/notmuch-mutt line 70.
> 
> If I remember correctly you need to use {} for hash references in Perl.

Woops! Thanks, fixed in the v2 patch series.

> It might clearer to use the return value from (a possibly renamed
> check_search_cache (then the -d can be eliminated / folded into the new
> function).

When the checks fail, the Maildir creation and Maildir symlink
additions shouldn't happen either, otherwise Maildirs with real data
may get mail files overwritten with symlinks back to themselves. So
there needs to be a die() or return before the creation and the search
anyway and so checks may as well just die() when the first check fails.

I experimented with returning anyway and it makes things more complex:

File::Find cannot abort the search except via die() and so the calling
code would need to use eval in order to catch the die() and return it.

Threading return values all the way back up to search_action adds lots
of error checks to every function along the way.

The search_action interactive mode doesn't do more than one search
anyway so it would still just need to die() when there is an error.

If the code were to grow a multi-command interactive prompt, that could
use eval to catch and print the die() errors. Such a prompt would need
to do that anyway, because the non-search actions call die() already.

So I think that throwing exceptions via die() is the simpler method.

Since checking that the Maildir is suitable is really a separate action
to clearing the previous search results, I've moved the check function
from the empty function to the search function in the v2 patch series.

-- 
bye,
pabs

https://bonedaddy.net/pabs3/
signature.asc (application/pgp-signature)
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: