[1/2] clang-format: Make Qt includes matching case sensitive
diff mbox series

Message ID 20240610180855.1643125-2-mzamazal@redhat.com
State Accepted
Headers show
Series
  • Includes ordering fix
Related show

Commit Message

Milan Zamazal June 10, 2024, 6:08 p.m. UTC
This fixes the problem that includes like

  #include <queue>

are put near the end.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 .clang-format | 1 +
 1 file changed, 1 insertion(+)

Comments

Laurent Pinchart June 10, 2024, 6:56 p.m. UTC | #1
Hi Milan,

Thank you for the patch.

On Mon, Jun 10, 2024 at 08:08:49PM +0200, Milan Zamazal wrote:
> This fixes the problem that includes like
> 
>   #include <queue>
> 
> are put near the end.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>  .clang-format | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.clang-format b/.clang-format
> index cac7029f..b48d4e1e 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -75,6 +75,7 @@ IncludeCategories:
>      Priority:        9
>    # Qt includes (match before C++ standard library)
>    - Regex:           '<Q([A-Za-z0-9\-_])+>'
> +    CaseSensitive:   true

This option is available in clang-format 12 and later. There are no
distribution versions we specifically care about that ship a too old
clang-formation version (Debian buster ships 11 by default but has a
clang-format-13 package), so this is fine. We should however update the
comment at the beginning of the file to indicate this. I can handle it
when applying this patch.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

By the way, see commit d1cdaeb6f078677d5bf46cb596102bfe8da070b9 :-)

>      Priority:        9
>    # Headers in <> with an extension. (+system libraries)
>    - Regex:           '<([A-Za-z0-9\-_])+\.h>'
Laurent Pinchart June 10, 2024, 7:01 p.m. UTC | #2
On Mon, Jun 10, 2024 at 09:56:42PM +0300, Laurent Pinchart wrote:
> Hi Milan,
> 
> Thank you for the patch.
> 
> On Mon, Jun 10, 2024 at 08:08:49PM +0200, Milan Zamazal wrote:
> > This fixes the problem that includes like

I will also expand this to

    Now that stable versions of all major distributions ship clang-format 12
    or newer, we can use the CaseSensitive option for the Qt include
    category. This fixes the problem that includes like

to add more context.

> > 
> >   #include <queue>
> > 
> > are put near the end.
> > 
> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> > ---
> >  .clang-format | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/.clang-format b/.clang-format
> > index cac7029f..b48d4e1e 100644
> > --- a/.clang-format
> > +++ b/.clang-format
> > @@ -75,6 +75,7 @@ IncludeCategories:
> >      Priority:        9
> >    # Qt includes (match before C++ standard library)
> >    - Regex:           '<Q([A-Za-z0-9\-_])+>'
> > +    CaseSensitive:   true
> 
> This option is available in clang-format 12 and later. There are no
> distribution versions we specifically care about that ship a too old
> clang-formation version (Debian buster ships 11 by default but has a
> clang-format-13 package), so this is fine. We should however update the
> comment at the beginning of the file to indicate this. I can handle it
> when applying this patch.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> By the way, see commit d1cdaeb6f078677d5bf46cb596102bfe8da070b9 :-)
> 
> >      Priority:        9
> >    # Headers in <> with an extension. (+system libraries)
> >    - Regex:           '<([A-Za-z0-9\-_])+\.h>'
Milan Zamazal June 11, 2024, 7:46 a.m. UTC | #3
Hi Laurent,

thank you for review.

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Hi Milan,
>
> Thank you for the patch.
>
> On Mon, Jun 10, 2024 at 08:08:49PM +0200, Milan Zamazal wrote:
>> This fixes the problem that includes like
>> 
>>   #include <queue>
>> 
>> are put near the end.
>> 
>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
>> ---
>>  .clang-format | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/.clang-format b/.clang-format
>> index cac7029f..b48d4e1e 100644
>> --- a/.clang-format
>> +++ b/.clang-format
>> @@ -75,6 +75,7 @@ IncludeCategories:
>>      Priority:        9
>>    # Qt includes (match before C++ standard library)
>>    - Regex:           '<Q([A-Za-z0-9\-_])+>'
>> +    CaseSensitive:   true
>
> This option is available in clang-format 12 and later. There are no
> distribution versions we specifically care about that ship a too old
> clang-formation version (Debian buster ships 11 by default but has a
> clang-format-13 package), so this is fine. We should however update the
> comment at the beginning of the file to indicate this. I can handle it
> when applying this patch.

Yes, please do.

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> By the way, see commit d1cdaeb6f078677d5bf46cb596102bfe8da070b9 :-)

I see, thanks for explanation.

>>      Priority:        9
>>    # Headers in <> with an extension. (+system libraries)
>>    - Regex:           '<([A-Za-z0-9\-_])+\.h>'

[...]

>> On Mon, Jun 10, 2024 at 08:08:49PM +0200, Milan Zamazal wrote:
>> > This fixes the problem that includes like
>
> I will also expand this to
>
>     Now that stable versions of all major distributions ship clang-format 12
>     or newer, we can use the CaseSensitive option for the Qt include
>     category. This fixes the problem that includes like
>
> to add more context.

OK, thank you.

>> > 
>> >   #include <queue>
>> > 
>> > are put near the end.

[...]
Kieran Bingham June 11, 2024, 12:03 p.m. UTC | #4
Quoting Milan Zamazal (2024-06-11 08:46:57)
> Hi Laurent,
> 
> thank you for review.
> 
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:
> 
> > Hi Milan,
> >
> > Thank you for the patch.
> >
> > On Mon, Jun 10, 2024 at 08:08:49PM +0200, Milan Zamazal wrote:
> >> This fixes the problem that includes like
> >> 
> >>   #include <queue>
> >> 
> >> are put near the end.
> >> 
> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> >> ---
> >>  .clang-format | 1 +
> >>  1 file changed, 1 insertion(+)
> >> 
> >> diff --git a/.clang-format b/.clang-format
> >> index cac7029f..b48d4e1e 100644
> >> --- a/.clang-format
> >> +++ b/.clang-format
> >> @@ -75,6 +75,7 @@ IncludeCategories:
> >>      Priority:        9
> >>    # Qt includes (match before C++ standard library)
> >>    - Regex:           '<Q([A-Za-z0-9\-_])+>'
> >> +    CaseSensitive:   true
> >
> > This option is available in clang-format 12 and later. There are no
> > distribution versions we specifically care about that ship a too old
> > clang-formation version (Debian buster ships 11 by default but has a
> > clang-format-13 package), so this is fine. We should however update the
> > comment at the beginning of the file to indicate this. I can handle it
> > when applying this patch.
> 
> Yes, please do.
> 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > By the way, see commit d1cdaeb6f078677d5bf46cb596102bfe8da070b9 :-)
> 
> I see, thanks for explanation.

Oh fantastic! I'm /very/ happy to see this reverted at last.
I tried making a system that would detect what version of clang-format
was available on the system and switch the .clang-format file - but it
was horrendous and wasn't really feasible.

Just moving to newer versions of clang-format is great from my
perspective ;D


> >>      Priority:        9
> >>    # Headers in <> with an extension. (+system libraries)
> >>    - Regex:           '<([A-Za-z0-9\-_])+\.h>'
> 
> [...]
> 
> >> On Mon, Jun 10, 2024 at 08:08:49PM +0200, Milan Zamazal wrote:
> >> > This fixes the problem that includes like
> >
> > I will also expand this to
> >
> >     Now that stable versions of all major distributions ship clang-format 12
> >     or newer, we can use the CaseSensitive option for the Qt include
> >     category. This fixes the problem that includes like
> >
> > to add more context.
> 
> OK, thank you.

All that sounds good to me.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> >> > 
> >> >   #include <queue>
> >> > 
> >> > are put near the end.
> 
> [...]
>

Patch
diff mbox series

diff --git a/.clang-format b/.clang-format
index cac7029f..b48d4e1e 100644
--- a/.clang-format
+++ b/.clang-format
@@ -75,6 +75,7 @@  IncludeCategories:
     Priority:        9
   # Qt includes (match before C++ standard library)
   - Regex:           '<Q([A-Za-z0-9\-_])+>'
+    CaseSensitive:   true
     Priority:        9
   # Headers in <> with an extension. (+system libraries)
   - Regex:           '<([A-Za-z0-9\-_])+\.h>'