mbox series

[v4,0/4] Add include guard checker and reformatter

Message ID 20240419150427.897379-1-tromey@adacore.com
Headers
Series Add include guard checker and reformatter |

Message

Tom Tromey April 19, 2024, 3:03 p.m. UTC 
  Here's v4 of the include guard checker / reformatter.

It's really just v2, but this time not sent using b4, which was
sending the wrong patches.

Tom

Comments

Simon Marchi April 19, 2024, 3:26 p.m. UTC | #1 
On 2024-04-19 11:03, Tom Tromey wrote:
> Here's v4 of the include guard checker / reformatter.
> 
> It's really just v2, but this time not sent using b4, which was
> sending the wrong patches.
> 
> Tom
> 

So, after patch 4/4, your script doesn't need to handle the the
`defined(FOO_H)` style (OLDDEF in your script).  I think you could
remove support for that, it would simplify it a little bit.

Either way:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon
Pedro Alves April 19, 2024, 4:55 p.m. UTC | #2 
On 2024-04-19 16:26, Simon Marchi wrote:
> So, after patch 4/4, your script doesn't need to handle the the
> `defined(FOO_H)` style (OLDDEF in your script).  I think you could
> remove support for that, it would simplify it a little bit.

Doesn't it need to handle it in case someone creates a new header and writes
defined(FOO_H) manually?  You'd still want the hook to fix that.
Simon Marchi April 19, 2024, 6:43 p.m. UTC | #3 
On 2024-04-19 12:55, Pedro Alves wrote:
> On 2024-04-19 16:26, Simon Marchi wrote:
>> So, after patch 4/4, your script doesn't need to handle the the
>> `defined(FOO_H)` style (OLDDEF in your script).  I think you could
>> remove support for that, it would simplify it a little bit.
> 
> Doesn't it need to handle it in case someone creates a new header and writes
> defined(FOO_H) manually?  You'd still want the hook to fix that.
> 

IMO, after the initial run, the script doesn't really need to auto fix
things, it can just check and report errors.

Simon
Tom Tromey April 19, 2024, 8:52 p.m. UTC | #4 
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 2024-04-19 12:55, Pedro Alves wrote:
>> On 2024-04-19 16:26, Simon Marchi wrote:
>>> So, after patch 4/4, your script doesn't need to handle the the
>>> `defined(FOO_H)` style (OLDDEF in your script).  I think you could
>>> remove support for that, it would simplify it a little bit.
>> 
>> Doesn't it need to handle it in case someone creates a new header and writes
>> defined(FOO_H) manually?  You'd still want the hook to fix that.
>> 

Simon> IMO, after the initial run, the script doesn't really need to auto fix
Simon> things, it can just check and report errors.

I had it work that way initially, but it was irritating to know that the
script could do this for me without my intervention, since it had
already figured out exactly what to do.

Tom
Pedro Alves April 26, 2024, 3:19 p.m. UTC | #5 
On 2024-04-19 19:43, Simon Marchi wrote:

> On 2024-04-19 12:55, Pedro Alves wrote:
>> On 2024-04-19 16:26, Simon Marchi wrote:
>>> So, after patch 4/4, your script doesn't need to handle the the
>>> `defined(FOO_H)` style (OLDDEF in your script).  I think you could
>>> remove support for that, it would simplify it a little bit.
>>
>> Doesn't it need to handle it in case someone creates a new header and writes
>> defined(FOO_H) manually?  You'd still want the hook to fix that.
>>
> 
> IMO, after the initial run, the script doesn't really need to auto fix
> things, it can just check and report errors.

The Black pre-hook fixes things for you instead of reporting formatting
issues.  I don't see why should this hook be held to different standards?