[PATCH] libcamera: formatting: Avoid spaces in for loops without expression

[Stanislaw Gruszka]

Hi Milan,

On Wed, Feb 26, 2025 at 10:10:01AM +0100, Milan Zamazal wrote:
> Hi Stanislaw,
> 
> Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com> writes:
> 
> > Hi,
> >
> > On Tue, Feb 25, 2025 at 11:14:07PM +0000, Kieran Bingham wrote:
> >> Quoting Laurent Pinchart (2025-02-25 21:22:03)
> >> > Hi Milan,
> >> > 
> >> > Thank you for the patch.
> >> > 
> >> > On Tue, Feb 25, 2025 at 04:28:06PM +0100, Milan Zamazal wrote:
> >> > > The clang formatter removes spaces in places of empty expressions in for
> >> > > loops.  For example, it changes
> >> > > 
> >> > >   for (init; condition; )
> >> > > 
> >> > > to
> >> > > 
> >> > >   for (init; condition;)
> >> > > 
> >> > > libcamera currently uses both the styles and we should use only one of
> >> > > them for consistency.  Since there is apparently no option to override
> >> > > the formatter behavior (see
> >> > > https://clang.llvm.org/docs/ClangFormatStyleOptions.html), let's remove
> >> > > the extra spaces to make the formatter happy.
> >> > > 
> >> > > Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> >> > 
> >> > It's not my favourite style, but I'm OK with it given the inconvenience
> >> > of making it a clang-format exception.
> >> 
> >> I remember we discussed this quite some time ago, but I'm fine with
> >> making the formatter happy - that makes all our lives easier.
> >
> > BTW I had formatter issue when editing src/libcamera/v4l2_device.cpp 
> > It get me changes like this:
> >
> > @@ -482,11 +482,12 @@ int V4L2Device::setFrameStartEnabled(bool enable)
> >  	if (frameStartEnabled_ == enable)
> >  		return 0;
> >  
> > -	struct v4l2_event_subscription event{};
> > +	struct v4l2_event_subscription event {
> > +	};
> 
> My clang formatter (19.1.7) is fine with the original version.

Interesting , I have 

Ubuntu clang-format version 19.1.1 (1ubuntu1~24.04.2)

and it does the change.

> >  void V4L2Device::eventAvailable()
> >  {
> > -	struct v4l2_event event{};
> > +	struct v4l2_event event {
> > +	};
> >  	int ret = ioctl(VIDIOC_DQEVENT, &event);
> >  	if (ret < 0) {
> >  		LOG(V4L2, Error)
> > @@ -850,11 +852,7 @@ void V4L2Device::eventAvailable()
> >  
> >  static const std::map<uint32_t, ColorSpace> v4l2ToColorSpace = {
> >  	{ V4L2_COLORSPACE_RAW, ColorSpace::Raw },
> > -	{ V4L2_COLORSPACE_SRGB, {
> > -		ColorSpace::Primaries::Rec709,
> > -		ColorSpace::TransferFunction::Srgb,
> > -		ColorSpace::YcbcrEncoding::Rec601,
> > -		ColorSpace::Range::Limited } },
> > +	{ V4L2_COLORSPACE_SRGB, { ColorSpace::Primaries::Rec709, ColorSpace::TransferFunction::Srgb, ColorSpace::YcbcrEncoding::Rec601, ColorSpace::Range::Limited } },
> 
> Generally, the formatter apparently prefers putting the initial brace on
> the next line.  In this case, this works to avoid the long line:

Good to know.

> 
> 	{ V4L2_COLORSPACE_SRGB,
> 	  { ColorSpace::Primaries::Rec709,
> 	    ColorSpace::TransferFunction::Srgb,
> 	    ColorSpace::YcbcrEncoding::Rec601,
> 	    ColorSpace::Range::Limited } },
> 
> But I'm not sure this is aligned with what the maintainers like.

For me personally both formats are ok.

> > and one such change
> >
> > -	for (unsigned int index = 0; ; index++) {
> > +	for (unsigned int index = 0;; index++) {
> >
> > what looks less convenient for me and perhaps is 
> > candidate for changing .clang-format rule if possible.
> 
> I personally mostly stopped caring about formatting after I entered the
> corporate world -- it's impossible to satisfy everybody and in the end
> result the code is unreadable for everybody but those that define the
> rules :-).  I use Emacs means to change the code presentation on the
> screen to avoid the worst offenders and learned to live with that.  An
> ideal situation is when a language provides its own canonical tools (see
> Rust, Go, ...) because it saves us from all the discussions.
> 
> The only really annoying part is when the formatter constantly changes
> edited sources and I have to discard the changes all the time.  So I
> welcome any changes (whether in .clang-format or in the source files)
> that make the formatter and the maintainers more compatible.

This annoys me a bit too.

I need to configure my editor - neovim to do autoformatting only to
lines that I changed and do not modify whole file. So far I was too 
lazy to do so.

Fortunately many files in libcamera are already formatted thanks to
paches like this one :-)

Thanks
Stanislaw