[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] CODING_STYLE: explicitly call out label indentation



On Mon, Nov 26, 2018 at 02:04:05AM -0700, Jan Beulich wrote:
> Since the behavior of "diff -p" to use an unindented label as context
> identifier often makes it harder to review patches, make explicit the
> requirement for labels to be indented.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -31,6 +31,10 @@ void fun(void)
>      }
>  }
>  
> +Due to the behavior of GNU diffutils "diff -p", labels should be
> +indented by at least one blank.  Non-case labels inside switch() bodies
> +are preferred to be indented the same as the block's case labels.
> +

Sorry, I don't follow this rationale.

I actually tried `diff -p` with and without indenting the label. Here is
the result.

With:

*** kernel.c.orig       2018-11-27 15:15:20.841296089 +0000
--- kernel.c    2018-11-27 15:20:23.192022064 +0000
*************** static int assign_integer_param(const st
*** 48,54 ****
      default:
          BUG();
      }
!
      return 0;
  }

--- 48,54 ----
      default:
          BUG();
      }
!  label:
      return 0;
  }


Without:

*** kernel.c.orig       2018-11-27 15:15:20.841296089 +0000
--- kernel.c    2018-11-27 15:21:01.456360128 +0000
*************** static int assign_integer_param(const st
*** 48,54 ****
      default:
          BUG();
      }
!
      return 0;
  }

--- 48,54 ----
      default:
          BUG();
      }
! label:
      return 0;
  }


They look the same to me. And frankly having an extra space in front make Xen
rather too unique. That's an issue for new comers and writing automated tool
to check patch.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.