[ros-kernel] Coding style/indentation (again!?)

Martin Fuchs martin-fuchs at gmx.net
Sat Jan 31 23:55:15 CET 2004


Sorry, if this mail may arrive you two times...


> > Revoking commits only because they don't conform to some
> > styling are also very problematic.
> 
> Why?

I can imagine situations, where the indenting/formating rules are not really
correct, and should be overidden. Or there may even be some bug, which
makes impossible to commit a change.

The main problem with this technical issues is, CVS is not laid out
for reformating in the middle of the commit process.
As you already explained, the changes on the CVS server are not
automatically propagated back to the commiter's file system.
This will always cause trouble - you don't see directly what has
happend, and will eventually get conflicts in following commits.


> > I tried your suggested indent command, and found a few 
> > problems with it.
> 
> What problems? Maybe I/we can fix/patch indent etc.

Well, I tried it with C++ code, and showed you the result.
This have have been "the problems" I talked about.

If you ask me what should be changed for formating C code, here
is an example:

<<<<<<<<<
   Entry = MmGetPageEntrySectionSegment(Segment, Offset);
   if (Entry == 0)
   {
      DPRINT1("Entry == 0 for MmUnsharePageEntrySectionSegment\n");
      KEBUGCHECK(0);
   }
   if (SHARE_COUNT_FROM_SSE(Entry) == 0)
   {
      DPRINT1("Zero share count for unshare\n");
      KEBUGCHECK(0);
   }
   if (IS_SWAP_FROM_SSE(Entry))
>>>>>>>>>

I am missing empty lines between the different "if" statements.

You can see this also in your example:

<<<<<<<<
   if (ptr && ((ptr[0] == ´\0 ´) || (ptr[1] == ´\0 ´ &&((ptr[0] == ´0 ´) || (ptr[0] == ´*´))))
       && ((ptr[0] == ´\0 ´) || (ptr[1] == ´\0 ´ &&((ptr[0] == ´0 ´) || (ptr[0] == ´*´)))))
   {
      printf("test");
   }
   switch (i)
   {
      case 1:
      {
         printf("1");
         break;
      }
      case 2:
         printf("1");
         break;
   }
   do
   {
      printf("do-while");
   }
   while (TRUE);
>>>>>>>>>

I suggest to insert one termination line before the "switch" and "do" lines.

I would change the following  function header into one line:

int
test(int arg1,
     int arg2,
     int arg3,
     int arg4)

But this is a matter of taste.


> > Also the manual page explains, indent doesn't handle C++ code.
> 
> Astyle supports C++ indentation: http://astyle.sourceforge.net/ 
> Currently I'm only interested in C files indentation thou. 

Yes, I know astyle - I already used it for C++ and Java code.
You can use astyle and indent successfully if you do it locally.
Just format the files with style problems in your copy of the tree, and
commit it to CVS. This is the easiest and least problematic solution.


Regards,

   Martin


--
Martin Fuchs
martin-fuchs at gmx.net



More information about the Ros-kernel mailing list