Thanks David, I've attached the modified patch that I think addresses all your comments -- I'm still not 100% sure about the whitespace though.

Thanks,

Lars


On Thu, 25 Jul 2024 17:33:48 +0900, David Bremner <david@tethera.net> wrote:
> Lars Kotthoff <lists@larsko.org> writes:
> 
> > Sorry, I dropped the ball on this -- here's the patch again with space/tab inconsistencies fixed. I wasn't entirely sure about this as it's inconsistent in the existing source, so I tried to make it as consistent as possible. Cover included again below.
> >
> 
> Hi Lars,
> 
> Thanks for sticking with this, and sorry I did not respond to the first
> thread. There are still one or two minor whitespace issues.  If you have
> "uncrustify" installed, then ./devel/check-notmuch-commit && git diff
> will find the ones I'm thinking of. You can ignore the problems you
> didn't introduce.
> 
> > The attached patch allows to customize the default ", " and "| "
> > separators in author lists. The main rationale for supporting this is
> > that the Python API uses the same functionality to get the list of
> > authors -- if I want to separate them again afterwards, I have to
> > split the returned string, which is error-prone with comma separators
> > (e.g. name in email address is of form Lastname, Firstname).
> 
> Can you add a brief summary of the motivation to the commit message?
> 
> >  test/T030-config.sh         |  2 ++
> >  test/T055-path-config.sh    |  4 ++++
> >  test/T590-libconfig.sh      | 10 ++++++++++
> 
> Thanks for adjusting the existing test suite.
> 
> Can you also add a simple test to check the functionality you
> introduced? Testing the CLI is fine, or if you prefer something in the
> style of test/T590-libconfig.sh
> 

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