diff mbox

Change arglist-intro default indentation in GDB .dir-locals.el

Message ID 1484336122-3804-1-git-send-email-antoine.tremblay@ericsson.com
State New
Headers show

Commit Message

Antoine Tremblay Jan. 13, 2017, 7:35 p.m. UTC
This changes the first line in an argument list indentation so that it is
indented one level rather than being aligned with the opening parens.

Before you would have:

function (
	  int a);

Now you will get:

function (
  int a);

This is done to accomodate situations where the line is very long and that
writing arguments aligned with the parens can't be done in 80 chars, like in
gdb/gdbserver/tracepoint.c:get_get_tsv_func_addr

CORE_ADDR
get_get_tsv_func_addr (void)
{
  CORE_ADDR res;
  if (read_inferior_data_pointer (
	ipa_sym_addrs.addr_get_trace_state_variable_value_ptr, &res))
    {
      error ("error extracting get_trace_state_variable_value_ptr");
      return 0;
    }
  return res;
}

There's multiple places were formatting like that is used already.  I
think we can make this a global policy.

Note that this does not affect the other arguments so writing a multiple
argument function call like:

void function (int a,
	       int b,
	       int c)

keeps the parens alignment.

Unless you start with an empty line which gives:

void function (
  int a,
  int b,
  int c)


gdb/ChangeLog:
	* .dir-locals.el: Add arg-list-intro indentation.
	* gdb-code-style.el (safe-local-variables-values): Add
	arg-list-intro +.
---
 gdb/.dir-locals.el    | 1 +
 gdb/gdb-code-style.el | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Pedro Alves Jan. 19, 2017, 4:06 p.m. UTC | #1
On 01/13/2017 07:35 PM, Antoine Tremblay wrote:
> This changes the first line in an argument list indentation so that it is
> indented one level rather than being aligned with the opening parens.
> 
> Before you would have:
> 
> function (
> 	  int a);
> 
> Now you will get:
> 
> function (
>   int a);
> 
> This is done to accomodate situations where the line is very long and that
> writing arguments aligned with the parens can't be done in 80 chars, like in
> gdb/gdbserver/tracepoint.c:get_get_tsv_func_addr
> 
> CORE_ADDR
> get_get_tsv_func_addr (void)
> {
>   CORE_ADDR res;
>   if (read_inferior_data_pointer (
> 	ipa_sym_addrs.addr_get_trace_state_variable_value_ptr, &res))
>     {
>       error ("error extracting get_trace_state_variable_value_ptr");
>       return 0;
>     }
>   return res;
> }
> 
> There's multiple places were formatting like that is used already.  I
> think we can make this a global policy.

Hmm, I don't think it's that common.  
Grepping around for "($" only finds a few hits.

Honestly, a "(" at the end of a line looks a bit odd in GNU-formatted code
to me.  I think the usual way is to break before the parens instead:

  if (read_inferior_data_pointer
      (ipa_sym_addrs.addr_get_trace_state_variable_value_ptr, &res))

> Note that this does not affect the other arguments so writing a multiple
> argument function call like:
> 
> void function (int a,
> 	       int b,
> 	       int c)

Doesn't look like a call.  :-)

> 
> keeps the parens alignment.
> 
> Unless you start with an empty line which gives:
> 
> void function (
>   int a,
>   int b,
>   int c)

Empty line?  ITYM, unless the first line has nothing after
the parens, right?

Thanks,
Pedro Alves
Matt Rice Jan. 20, 2017, 12:03 a.m. UTC | #2
On Thu, Jan 19, 2017 at 8:06 AM, Pedro Alves <palves@redhat.com> wrote:
> On 01/13/2017 07:35 PM, Antoine Tremblay wrote:
>> This changes the first line in an argument list indentation so that it is
>> indented one level rather than being aligned with the opening parens.
>>
>> Before you would have:
>>
>> function (
>>         int a);
>>
>> Now you will get:
>>
>> function (
>>   int a);
>>
>> This is done to accomodate situations where the line is very long and that
>> writing arguments aligned with the parens can't be done in 80 chars, like in
>> gdb/gdbserver/tracepoint.c:get_get_tsv_func_addr
>>
>> CORE_ADDR
>> get_get_tsv_func_addr (void)
>> {
>>   CORE_ADDR res;
>>   if (read_inferior_data_pointer (
>>       ipa_sym_addrs.addr_get_trace_state_variable_value_ptr, &res))
>>     {
>>       error ("error extracting get_trace_state_variable_value_ptr");
>>       return 0;
>>     }
>>   return res;
>> }
>>
>> There's multiple places were formatting like that is used already.  I
>> think we can make this a global policy.
>
> Hmm, I don't think it's that common.
> Grepping around for "($" only finds a few hits.
>
> Honestly, a "(" at the end of a line looks a bit odd in GNU-formatted code
> to me.  I think the usual way is to break before the parens instead:
>
>   if (read_inferior_data_pointer
>       (ipa_sym_addrs.addr_get_trace_state_variable_value_ptr, &res))
>
>> Note that this does not affect the other arguments so writing a multiple
>> argument function call like:
>>
>> void function (int a,
>>              int b,
>>              int c)
>
> Doesn't look like a call.  :-)
>
>>
>> keeps the parens alignment.
>>
>> Unless you start with an empty line which gives:
>>
>> void function (
>>   int a,
>>   int b,
>>   int c)
>
> Empty line?  ITYM, unless the first line has nothing after
> the parens, right?

FWIW, i've always preferred applying the same rule as is typically
used with bitwise operators to commas and the like... that is , goes
on the beginning of the next line rather than end of the first and
would format it like

void function
  (int a
  ,int b
  ,int c)

in addition to the 'break before parens' that Pedro mentions...
I can completely understand if nobody likes this approach though...
Antoine Tremblay Jan. 23, 2017, 12:14 p.m. UTC | #3
Pedro Alves writes:

> Hmm, I don't think it's that common.  
> Grepping around for "($" only finds a few hits.
>
> Honestly, a "(" at the end of a line looks a bit odd in GNU-formatted code
> to me.  I think the usual way is to break before the parens instead:
>
>   if (read_inferior_data_pointer
>       (ipa_sym_addrs.addr_get_trace_state_variable_value_ptr, &res))

OK. I keep hesitating between the 2 but that settles it, thanks for
reviewing.
diff mbox

Patch

diff --git a/gdb/.dir-locals.el b/gdb/.dir-locals.el
index a2aa196..8e0503a 100644
--- a/gdb/.dir-locals.el
+++ b/gdb/.dir-locals.el
@@ -25,5 +25,6 @@ 
 	    (tab-width . 8)
 	    (c-basic-offset . 2)
 	    (eval . (c-set-offset 'innamespace 0))
+	    (eval . (c-set-offset 'arglist-intro '+))
 	    ))
 )
diff --git a/gdb/gdb-code-style.el b/gdb/gdb-code-style.el
index 8cdec77..bd1c8d3 100644
--- a/gdb/gdb-code-style.el
+++ b/gdb/gdb-code-style.el
@@ -78,5 +78,8 @@ 
 (setq safe-local-variable-values
       (cons safe-local-variable-values
 	    '((eval c-set-offset 'innamespace 0)
-	      (c-offsets-alist (innamespace . 0)))))
+	      (c-offsets-alist (innamespace . 0))
+	      (eval c-set-offset 'arglist-intro '+)
+	      (c-offsets-alist (arglist-intro . '+))
+	      )))
 ;;; gdb-code-style.el ends here