Re: [PATCH v2 5/7] lib: return NOTMUCH_STATUS_OPERATION_INVALIDATED where appropriate

Subject: Re: [PATCH v2 5/7] lib: return NOTMUCH_STATUS_OPERATION_INVALIDATED where appropriate

Date: Tue, 05 Aug 2025 08:27:27 +0200

To: notmuch@notmuchmail.org, David Bremner

Cc: xapian-discuss@lists.xapian.org

From: Anton Khirnov


Quoting David Bremner (2025-08-04 16:11:31)
> 
> I have applied the first 4 patches in the series to master.

Nice, thank you!

> >  	notmuch->exception_reported = true;
> > -	ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
> > +	ret = _notmuch_xapian_error(error);
> 
> devel/STYLE says there needs to be a space after _notmuch_xapian_error
> here. uncrustify -c devel/uncrustify.cfg can (mostly) automate this.

Sorry that I keep doing this, I'm very used to the style without a
space.

> > --- a/lib/message.cc
> > +++ b/lib/message.cc
> > @@ -297,7 +297,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch,
> >  			       "A Xapian exception occurred creating message: %s\n",
> >  			       error.get_msg ().c_str ());
> >  	notmuch->exception_reported = true;
> > -	*status_ret = NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION;
> > +	*status_ret = _notmuch_xapian_errorp(error);
> 
> please err on the side verbosity when adding new (private) API functions.

I assume you meant in the function name?

> > +static inline notmuch_private_status_t
> > +_notmuch_xapian_errorp (const Xapian::Error &error)
> > +{
> > +    const char *type = error.get_type();
> > +	return (! strcmp (type, "DatabaseModifiedError")) ?
> > +	    NOTMUCH_PRIVATE_STATUS_OPERATION_INVALIDATED : NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION;
> > +}
> 
> My first reaction to using strcmp here is ugh, but I guess that's the
> intended method of introspection. I have CC'ed xapian-users, in case
> someone there wants to comment. Naively, it seems like a case where
> typeid would be useful, but I am not enough of a C++ expert to
> understand why Xapian doesn't do that.

I'm not all that happy about it either, but I don't see another way to
share the cleanup code. But then again my C++ is very basic.

-- 
Anton Khirnov
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: