[dpdk-dev] [PATCH] doc: coding style: use linux kernel style for indentation

[dpdk-dev] [PATCH] doc: coding style: use linux kernel style for indentation

[Yuanhan Liu]

Using two tabs for "if" (or "while") statements is a bit weird to me.
Also, using one tab unconditionaly for function definitions and
prototypes doesn't look great.

Here I'd suggest to use the indentation style the Linux kernel
project prefers: to align with the open brace with tabs and
additonal spaces (when necessary).

Example:

  static int
  rte_eal_foo_bar(int a_long_argument_1, int another_long_argument_2,
                  struct foo *yet_another_long_argument_3)

  ret = rte_eal_foo_bar(a_long_argument_1, another_long_argument_2,
                        yet_another_long_argument_3);

  if (really_long_variable_name_1 == really_long_variable_name_2 &&
      var3 == var4) {
          x = y + z;
          ....;
  }

Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: Siobhan Butler <siobhan.a.butler@intel.com>
Cc: John McNamara <john.mcnamara@intel.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 doc/guides/contributing/coding_style.rst | 38 ++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index ad1392d..23cd060 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -339,9 +339,11 @@ General
 
 * Do not put any spaces before a tab for indentation.
 * If you have to wrap a long statement, put the operator at the end of the line, and indent again.
-* For control statements (if, while, etc.), continuation it is recommended that the next line be indented by two tabs, rather than one,
-  to prevent confusion as to whether the second line of the control statement forms part of the statement body or not.
-  Alternatively, the line continuation may use additional spaces to line up to an appropriately point on the preceding line, for example, to align to an opening brace.
+* For control statements (if, while, etc.), function definitions and
+  function prototypes continuation lines, it is recommended that the
+  next line be indented by tab and additional spaces when necessary
+  to align to the opening brace. For others, it's also okay to be
+  indented by tab only.
 
 .. note::
 
@@ -350,17 +352,29 @@ General
 .. code-block:: c
 
  while (really_long_variable_name_1 == really_long_variable_name_2 &&
-     var3 == var4){  /* confusing to read as */
-     x = y + z;      /* control stmt body lines up with second line of */
-     a = b + c;      /* control statement itself if single indent used */
+         var3 == var4) { /* confusing to read as */
+         x = y + z;      /* control stmt body lines up with second line of */
+         a = b + c;      /* control statement itself if single indent used */
  }
 
  if (really_long_variable_name_1 == really_long_variable_name_2 &&
-         var3 == var4){  /* two tabs used */
-     x = y + z;          /* statement body no longer lines up */
-     a = b + c;
+     var3 == var4) {            /* align with above opening if statement */
+         x = y + z;             /* statement body no longer lines up */
+         a = b + c;
  }
 
+ /* The continuation line is indented with two tab + 1 space */
+ static int
+ rte_eal_foo_bar(int a_long_argument_1, int another_long_argument_2,
+                 struct foo *yet_another_long_argument_3)
+ {
+        ...
+ }
+
+ /* The continuation line is indented with two tab + 7 spaces */
+ ret = rte_eal_foo_bar(a_long_argument_1, another_long_argument_2,
+                       yet_another_long_argument_3);
+
  z = a + really + long + statement + that + needs +
          two + lines + gets + indented + on + the +
          second + and + subsequent + lines;
@@ -510,9 +524,9 @@ Prototypes
 .. code-block:: c
 
  static char *function1(int _arg, const char *_arg2,
-        struct foo *_arg3,
-        struct bar *_arg4,
-        struct baz *_arg5);
+                        struct foo *_arg3,
+                        struct bar *_arg4,
+                        struct baz *_arg5);
  static void usage(void);
 
 .. note::
-- 
1.9.0

Re: [dpdk-dev] [PATCH] doc: coding style: use linux kernel style for indentation

[Bruce Richardson]

On Wed, Jan 13, 2016 at 03:58:49PM +0800, Yuanhan Liu wrote:
> Using two tabs for "if" (or "while") statements is a bit weird to me.
> Also, using one tab unconditionaly for function definitions and
> prototypes doesn't look great.
> 
> Here I'd suggest to use the indentation style the Linux kernel
> project prefers: to align with the open brace with tabs and
> additonal spaces (when necessary).
> 
> Example:
> 
>   static int
>   rte_eal_foo_bar(int a_long_argument_1, int another_long_argument_2,
>                   struct foo *yet_another_long_argument_3)
> 
>   ret = rte_eal_foo_bar(a_long_argument_1, another_long_argument_2,
>                         yet_another_long_argument_3);
> 
>   if (really_long_variable_name_1 == really_long_variable_name_2 &&
>       var3 == var4) {
>           x = y + z;
>           ....;
>   }
> 
> Cc: Thomas Monjalon <thomas.monjalon@6wind.com>
> Cc: Siobhan Butler <siobhan.a.butler@intel.com>
> Cc: John McNamara <john.mcnamara@intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---

While it's not a big deal - yet is something likely to trigger massive discussion
my objections to this style of indentation is two-fold:

1. It means that we are using a mix of tabs and spaces for indentation at the 
start of a line. I think it's more consistent that all whitespace at the start
of a line should be either tabs or spaces, but not a mixture.
2. It makes how the code look much more dependent upon the tab-size being used
in the viewer - being able to adjust how much whitespace is seem at the start of
each line is a major advantage of using tabs rather than spaces for indentation.
For anyone using a 4-character tabstop - probably the most popular tabstop value
after an 8-char one - aligning an if-statement continuation to the opening brace
will cause it to align with the body of the block.

So, while the two-tab indent may look "a bit weird" it does solve the two issues
above. I believe practical benefits should override initial impressions. [It took
me a while to get used to also, but now I very much like it as a style.]

Regards,
/Bruce

Re: [dpdk-dev] [PATCH] doc: coding style: use linux kernel style for indentation

[Stephen Hemminger]

On Wed, 13 Jan 2016 15:07:08 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> So, while the two-tab indent may look "a bit weird" it does solve the two issues
> above. I believe practical benefits should override initial impressions. [It took
> me a while to get used to also, but now I very much like it as a style.]

I don't think that deviating from kernel style for this case is justified.

Re: [dpdk-dev] [PATCH] doc: coding style: use linux kernel style for indentation

[Ferruh Yigit]

On 1/13/2016 4:49 PM, stephen at networkplumber.org (Stephen Hemminger) wrote:
> On Wed, 13 Jan 2016 15:07:08 +0000
> Bruce Richardson <bruce.richardson at intel.com> wrote:
> 
>> So, while the two-tab indent may look "a bit weird" it does solve the two issues
>> above. I believe practical benefits should override initial impressions. [It took
>> me a while to get used to also, but now I very much like it as a style.]
> 
> I don't think that deviating from kernel style for this case is justified.

This is very old patch still sitting in patchwork, re-visiting it mainly to be
able to clean the patchwork.

This is a syntax change request and although I have my personal preferences I
would be OK with whatever decided.

Currently there is already a decided syntax, changing it after this point will
cause either mixed usage or a big syntax cleanup patch. I think both are not good.

I am for continue whatever documented in current DPDK coding style doc, hence
NAK from my side.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] doc: coding style: use linux kernel style for indentation

[Yuanhan Liu]

On Tue, Feb 13, 2018 at 12:51:57PM +0000, Ferruh Yigit wrote:
> On 1/13/2016 4:49 PM, stephen at networkplumber.org (Stephen Hemminger) wrote:
> > On Wed, 13 Jan 2016 15:07:08 +0000
> > Bruce Richardson <bruce.richardson at intel.com> wrote:
> > 
> >> So, while the two-tab indent may look "a bit weird" it does solve the two issues
> >> above. I believe practical benefits should override initial impressions. [It took
> >> me a while to get used to also, but now I very much like it as a style.]
> > 
> > I don't think that deviating from kernel style for this case is justified.
> 
> This is very old patch still sitting in patchwork, re-visiting it mainly to be
> able to clean the patchwork.

Just drop it, there is no reason to revisit it.

	--yliu
> 
> This is a syntax change request and although I have my personal preferences I
> would be OK with whatever decided.
> 
> Currently there is already a decided syntax, changing it after this point will
> cause either mixed usage or a big syntax cleanup patch. I think both are not good.
> 
> I am for continue whatever documented in current DPDK coding style doc, hence
> NAK from my side.
> 
> Thanks,
> ferruh