[PATCH v2 4/5] libcamera: base: Fix formatting in thread.cpp

[Laurent Pinchart]

On Mon, Feb 24, 2025 at 09:34:28PM +0100, Milan Zamazal wrote:
> Laurent Pinchart writes:
> > On Mon, Feb 24, 2025 at 07:52:34PM +0100, Milan Zamazal wrote:
> >> Let's make the autoformatter happy with thread.cpp formatting.
> >> 
> >> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> >> ---
> >>  src/libcamera/base/thread.cpp | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> >> index 319bfda9..02128f23 100644
> >> --- a/src/libcamera/base/thread.cpp
> >> +++ b/src/libcamera/base/thread.cpp
> >> @@ -5,8 +5,6 @@
> >>   * Thread support
> >>   */
> >>  
> >> -#include <libcamera/base/thread.h>
> >> -
> >
> > This is done on purpose, see Documentation/coding-style.rst:
> >
> >   For .cpp files, if the file implements an API declared in a header
> >   file, that header file shall be included first in order to ensure it
> >   is self-contained.
> 
> Ah, right.  It seems the autoformatter does the right thing for includes
> with quotes but not for those with angle brackets.  I can see in
> .clang-format
> 
>   IncludeCategories:
>     # Headers matching the name of the component are matched automatically.
>     # Priority 1
> 
> so it doesn't look like we can do anything about it.
> 
> >>  #include <atomic>
> >>  #include <list>
> >>  #include <optional>
> >> @@ -20,6 +18,7 @@
> >>  #include <libcamera/base/message.h>
> >>  #include <libcamera/base/mutex.h>
> >>  #include <libcamera/base/object.h>
> >> +#include <libcamera/base/thread.h>
> >>  
> >>  /**
> >>   * \page thread Thread Support
> >> @@ -657,7 +656,7 @@ void Thread::dispatchMessages(Message::Type type)
> >>  	 * the outer calls.
> >>  	 */
> >>  	if (!--data_->messages_.recursion_) {
> >> -		for (auto iter = messages.begin(); iter != messages.end(); ) {
> >> +		for (auto iter = messages.begin(); iter != messages.end();) {
> >
> > Hmmmm... I thought we standardized on the style with a space, but we
> > have 6 occurrences with a space, and 15 without. Among those 15, 10 are
> > in the software ISP though :-)
> 
> Maybe because of the autoformatter assistance?
> 
> > I find the space more readable, but if there's no way to make
> > clang-format happy about it, I'm OK dropping it.
> 
> I don't mind either way as long as I don't have to override changes made
> by the formatter all the time. :-)

I didn't find a suitable clang-format option after a cursory look. If
you can't see one either, I'm fine changing this.

As the header change from this patch needs to be dropped, you could
submit a v2 that drops the space after the semicolon in all locations (I
just ran "git grep '; )'" to locate them). That can be sent as patch of
its own, separate from this series.

> >>  			if (!*iter)
> >>  				iter = messages.erase(iter);
> >>  			else

-- 
Regards,

Laurent Pinchart