[PATCH] Add utility to check for indentation in files or patches

[Simo]

On Tue, 2018-04-10 at 10:30 +1200, Andrew Bartlett via samba-technical
wrote:
> On Mon, 2018-04-09 at 18:18 -0400, Simo via samba-technical wrote:
> > Attached find a new version that deals with python3 properly and
> > also has better handling of indent, by spitting only a diff file with
> > the propsed diff you should apply to pass it's rules.
> > Previously the code was trying to line match and give a generic error
> > with the line number but that doesn't work because indent can change
> > lines by wrapping or unwrapping so the line numbers would make no sense
> >  on longer files.
> > 
> > I have no yet addressed the space and parenthesized issues in this
> > specific email simply because I ran out of time and I am not sure we
> > actually do not want them.
> > 
> > I am open to changing these though if more people agree on the need for
> > changing them.
> > 
> 
> G'Day Simo,
> 
> Thank you so much for taking charge of this.  
> 
> I wondered if the check-file script could be re-worked into a subunit test?  

I do not know, check-file can check a whole file, but I am not sure we
want to do it right away.

> It would be great if we had an idea how much of our code actually
> complies with README.Coding.  

You can do it easily with a find/xargs script that calls check-file.py
on each file, so far I haven't found a single file that complies :-)

> I'm thinking that if we wrote a rule to run the check-file script
> against each .c and .h file in the tree, encoding the file name and
> rule into the test output we could then handle exceptions via our skip,
> knownfail and flapping process.

Keep in mind we do not check the whole file on commit, we check the
diff, so that existing violations are not flagged.

> I'm hoping that would make it easier to add exceptions when files are
> renamed or functions re-ordered, without absolutely requiring that the
> code be re-formatted there and then.  

I think it is a good idea to always have style correction on file
renames and function reorders (as separate commits of course), it
allows us to gradually fix our code.

Making exceptions for those is not as easy, and I am not sure how the
implementation would look like for function moves (file renames are
probably easy enough).

> If we actually tested the full tree we could perhaps fix up some of the
> more egregious examples, particularly where it is just whitespace (and
> so can be verified as harmless with a whitespace-ignoring diff).

I think we can try to achieve these goals, but as follow up work, I
would like to start having something now rather than try to shoot for
everything at once.

Simo.

> Thanks!
> 
> Andrew Bartlett
> 
> > 
> > On Mon, 2018-04-09 at 15:35 -0400, Simo via samba-technical wrote:
> > > On Mon, 2018-04-09 at 22:17 +1000, Martin Schwenke wrote:
> > > > On Mon, 09 Apr 2018 07:51:56 -0400, Simo <simo at samba.org> wrote:
> > > > 
> > > > > On Mon, 2018-04-09 at 20:48 +1000, Martin Schwenke wrote:
> > > > > > It just seems to blow up on some commits with a traceback.  Can someone
> > > > > > please explain what I'm doing wrong?  
> > > > > 
> > > > > Do you have 'indent' installed ?
> > > > > I probably need to catch the exception better, I changed this part from
> > > > > the original stuff, as it was using emacs ...
> > > > 
> > > > I didn't but I do now.  Much better!  Thanks!  :-)
> > > > 
> > > > A couple of things that we might not want:
> > > > 
> > > > * No space before or after binary operator
> > > > 
> > > >   We have "good" examples in README.Coding without spaces around binary
> > > >   operators.  For example, in a for-loop and in a calculation involving
> > > >   sizeof().
> > > > 
> > > > * Parenthesized return expression
> > > > 
> > > >   We don't have a rule against this.
> > > > 
> > > >   In all the simple cases, doing this is bad style.  However, for a
> > > >   complex expression it might provide clarity.
> > > > 
> > > >   For example, in the same way that:
> > > > 
> > > >     x = (y.foo == z.bar);
> > > > 
> > > >   gains clarity from the parentheses, I think the following does too:
> > > > 
> > > >     return (y.foo == z.bar);
> > > > 
> > > >   Or, perhaps, the latter is against the guidelines because we should
> > > >   be doing:
> > > > 
> > > >     ok = (y.foo == z.bar);
> > > >     return ok;
> > > > 
> > > >   However, since we have access to y.foo and z.bar in a debugger, we
> > > >   can easily check what the return value will be, so I think the
> > > >   original is OK.
> > > > 
> > > >   Not sure...  :-)
> > > 
> > > 
> > > Thanks,
> > > I am updating the code to work with python3 (the original catered for
> > > python 2.6 ...) and I will add measures to deal with your feedback too,
> > > although I have to say I think I may prefer to keep both rules above as
> > >  that's what indent also does ?
> > > Otherwise I need to check how to suppress that in indent I guess.
> > > 
> > > But I'll look into it.
> > > 
> > > Simo.