diff mbox series

[Openvpn-devel,2/2] uncrustify: remove newlines after return type of function prototype

Message ID 20220818201505.656149-2-frank@lichtenheld.com
State Rejected
Headers
Series [Openvpn-devel,1/2] reformat: remove newline after return type of function prototype |

Commit Message

Frank Lichtenheld Aug. 18, 2022, 10:15 a.m. UTC 
  Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
 dev-tools/uncrustify.conf | 1 +
 1 file changed, 1 insertion(+)

Comments

Antonio Quartulli Aug. 18, 2022, 10:43 a.m. UTC | #1 
yes! This is what we need!

Acked-by: Antonio Quartulli <a@unstable.cc>
Arne Schwabe Aug. 18, 2022, 11:50 p.m. UTC | #2 
Am 18.08.22 um 22:43 schrieb Antonio Quartulli:
> yes! This is what we need!
> 
> Acked-by: Antonio Quartulli <a@unstable.cc>
> 

I am out of the loop here. Could you two explain why we need this? I.e. 
what is wrong with the current style is and what the plan is to change?

If often struggle to properly format function with the soft limit of 80 
without being allowed to put the return type on a separate line. So I am 
not sure what the improvement of this change is.

Arne
Antonio Quartulli Aug. 19, 2022, 12:10 a.m. UTC | #3 
Hi,

On 19/08/2022 11:50, Arne Schwabe wrote:
> Am 18.08.22 um 22:43 schrieb Antonio Quartulli:
>> yes! This is what we need!
>>
>> Acked-by: Antonio Quartulli <a@unstable.cc>
>>
> 
> I am out of the loop here. Could you two explain why we need this? I.e. 
> what is wrong with the current style is and what the plan is to change?
> 
> If often struggle to properly format function with the soft limit of 80 
> without being allowed to put the return type on a separate line. So I am 
> not sure what the improvement of this change is.
> 

We are not changing the style here.

Basically we have originally started using this approach (when we 
discussed the style back with Steffan some years ago):

* prototypes: return type on the same line as the function name (makes 
the first line a bit longer, but normally is not a big deal). Should the 
line be too long to make the name fit within the 100chars limit we have, 
you can still go to a new line, but should really be an exception.

* function definitions: return type on its own line.

Basically it's just a matter of style, not convenience.

Note: this has been like this since a while. Only a few occurrences were 
not respecting this style (and Frank is fixing those in 1/2).

Cheers,

> Arne
Gert Doering Aug. 19, 2022, 12:18 a.m. UTC | #4 
Hi,

On Fri, Aug 19, 2022 at 11:50:04AM +0200, Arne Schwabe wrote:
> Am 18.08.22 um 22:43 schrieb Antonio Quartulli:
> > yes! This is what we need!
> > 
> > Acked-by: Antonio Quartulli <a@unstable.cc>
> 
> I am out of the loop here. Could you two explain why we need this? I.e. 
> what is wrong with the current style is and what the plan is to change?
> 
> If often struggle to properly format function with the soft limit of 80 
> without being allowed to put the return type on a separate line. So I am 
> not sure what the improvement of this change is.

This is the conclusion that I arrived to as well, when reviewing the
changes in 1/2.  Forcing the prototype on the return type while at
the same time permitting enums, structs, etc., will cause cases like

enum some_special_type special_function_with_descriptive_name(int arg1,
                                                              char * arg2);

which is definitely less readable than 

enum some_special_type 
special_function_with_descriptive_name(int arg1, char * arg2);

or maybe

enum some_special_type 
  special_function_with_descriptive_name(int arg1, char * arg2);


Now, for other cases,

int
somefunc(void);

I find the other style

int somefunc(void);

much easier to grok ("have a single look and see what this is about").


... so, my suggestion is that we permit both styles for function prototypes,
based on overall type + function name + parameter length.

It would be cool if uncrustify had a magic flag for that, like "if 
return type + function name is < 40 characters, put on a single line,
and if over, split" :-)

Am I making sense?

gert
Frank Lichtenheld Aug. 24, 2022, 1:42 a.m. UTC | #5 
On Fri, Aug 19, 2022 at 12:18:06PM +0200, Gert Doering wrote:
> Hi,
[...] 
> It would be cool if uncrustify had a magic flag for that, like "if 
> return type + function name is < 40 characters, put on a single line,
> and if over, split" :-)

Pretty sure there is nothing like that. So should we mark this patch
just as rejected in patchwork and move on?

> Am I making sense?

I understand your reasoning. I wouldn't agree that it is worth clinging
to a style that can't be enforced just because it is slightly prettier.
But definitely not going to waste further time arguing about this
specific bikeshed ;)

Regards,
diff mbox series

Patch

diff --git a/dev-tools/uncrustify.conf b/dev-tools/uncrustify.conf
index 325f3108..c73fba0c 100644
--- a/dev-tools/uncrustify.conf
+++ b/dev-tools/uncrustify.conf
@@ -40,6 +40,7 @@  sp_after_comma=add
 pos_arith=Lead
 pos_bool=Lead
 nl_func_type_name=add
+nl_func_proto_type_name=remove
 nl_before_case=true
 nl_assign_leave_one_liners=true
 nl_enum_leave_one_liners=true