Re: [squid-dev] Coding Style updates
On 8/15/21 9:07 PM, Amos Jeffries wrote: > The existence of such a style requirement on Factory developers, and > thus need for Squid code to match it for ease of future bug fixing, was > given to me as a reason for ICAP and eCAP feature code staying in the > Factory supplied one-line format despite the remainder of Squid code > back then using two-line. The above interpretation does not match reality. I continue to urge you to stop describing things that you do not have enough information about to describe accurately, especially things that relate to other people. Statistically speaking, the probability of success for such dangerous attempts has been close to zero (and there is no actual need to describe those things to advance your arguments). > So, between your two responses I gather that there will be no push-back > on a PR adding enforcement of two-line function/method definitions by > astyle 3.1. There is push back, of course -- any large format changes need to justify the expenses they necessarily bring. Whether that push back is enough to block that future PR depends, AFAICT, on Astyle formatting results and the timing of migration to auto results. As previously discussed, we do not want to reformat the same large set of lines twice in a short succession. > It appears to be one of the policy rules not copied over to that page > from the Squid-2 page. The fact that the alleged official formatting rules are not actually documented anywhere explains some of the inconsistencies and makes making future rule setting easier IMO. Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] Coding Style updates
On 15/08/21 3:44 am, Alex Rousskov wrote: On 8/12/21 8:31 PM, Amos Jeffries wrote: I am aware that Factory ... prefers the one-line style. Factory does not prefer the one-line style. The existence of such a style requirement on Factory developers, and thus need for Squid code to match it for ease of future bug fixing, was given to me as a reason for ICAP and eCAP feature code staying in the Factory supplied one-line format despite the remainder of Squid code back then using two-line. So, between your two responses I gather that there will be no push-back on a PR adding enforcement of two-line function/method definitions by astyle 3.1. If we don't have agreement on a change I will implement enforcement of the existing style policy. I cannot find any existing/official rules regarding single- or multi-line function definitions in [1]. Where are they currently stated? [1] https://wiki.squid-cache.org/SquidCodingGuidelines It appears to be one of the policy rules not copied over to that page from the Squid-2 page. "Follow the coding style of the rest of the code." ... the bulk of Squid code uses two-line. Only the ICAP,eCAP, SSL-Bump code received in large PRs from Factory or third-party imported libraries (also large imports) use one-line. Amos ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] Coding Style updates
On 8/12/21 8:31 PM, Amos Jeffries wrote: > I am aware that Factory ... prefers the one-line style. Factory does not prefer the one-line style. > If we don't have agreement on a change I will > implement enforcement of the existing style policy. I cannot find any existing/official rules regarding single- or multi-line function definitions in [1]. Where are they currently stated? [1] https://wiki.squid-cache.org/SquidCodingGuidelines Whether we should (adopt, if it has not been already, and) enforce a particular style depends, in part, on how well Astyle supports that style. The quality of that support will probably become clear from the corresponding PR, and we can decide what to do at PR review time. Alex. ___ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] Coding Style updates
On 13/08/21 4:28 am, Alex Rousskov wrote:
On 8/12/21 12:42 AM, Amos Jeffries wrote:
1) return type on separate line from function definition.
Current style requirement:
template<...>
void
foo(...)
{
...
}
AFAIK, this based on GNU project style preferences from the far past
when Squid had a tight relationship with GNU. Today we have a conflict
between Factory code coming in with the alternative same-line style
and/or various developers being confused by the mix of code they are
touching.
AFAIK, Factory developers try to follow the official style (while
keeping the old code consistent), just like all the other developers
(should) do. They make mistakes just like all other developers do. It is
unfortunate that you felt it is necessary to single out a group of
developers in a negative way (that is completely irrelevant to your
actual proposal).
I don't mean that as a bad thing. Just that I am aware that Factory is a
large contributor who prefers the one-line style.
IMO; it is easier to work with the one-line style when command-line
editing, and irrelevant when using more advanced tools.
FWIW, I do not find it easier to use one-line style when using
command-line tools. In fact, the opposite is probably true in my experience.
Hmm. Okay.
As such I propose converting Squid to the same-line style:
template<...>
void foo(...)
{
...
}
Opinions?
One-line stile increases horizontal wrapping, especially when the return
type is complex/long and there are additional markers like "static",
"inline", and "[[ noreturn ]]".
One line approach itself is inconsistent because template<> is on the
other line(s).
Searching for a function name with unknown-to-searcher or complex return
type becomes a bit more difficult when using "git log -L" and similar tools.
In many cases, function parameters should be on multiple lines as well.
Their alignment would look worse if the function name does not start the
line.
Most function definitions will have "auto" return type after we upgrade
to C++14. Whether that makes one-line style more or less attractive is
debatable, but it is an important factor in delaying related changes.
Eventually, we may be able to automatically remove explicit return types
using one of the clang-tidy tools, but such a tool does not exist yet
(modernize-use-trailing-return-type comes close but is not it).
In summary, I recommend delaying this decision until C++14. However, if
others insist on changing the format and changing it now, then I will
not block these changes, assuming newer astyle produces reasonable results.
Can new astyle support multiline formatting? If not, _that_ could be a
strong argument for changing the style.
It can support both now. If we don't have agreement on a change I will
implement enforcement of the existing style policy.
2) braces on one-liner conditional blocks
Current code style is a bit confused. We have it as a style to use, with
exceptions for some old macros etc where the lack of braces causes
compile issues or bugs.
Personally I prefer the non-braced style. But it seems far safer to
automate the always-brace style and not have those special exceptions.
Opinions?
I also prefer the non-braced style for simple expressions.
Perhaps there is a way to automatically wrap complex ones, for some
reasonable definition of "complex"?
Alex.
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] Coding Style updates
On 8/12/21 12:42 AM, Amos Jeffries wrote:
> 1) return type on separate line from function definition.
>
> Current style requirement:
>
> template<...>
> void
> foo(...)
> {
> ...
> }
>
> AFAIK, this based on GNU project style preferences from the far past
> when Squid had a tight relationship with GNU. Today we have a conflict
> between Factory code coming in with the alternative same-line style
> and/or various developers being confused by the mix of code they are
> touching.
AFAIK, Factory developers try to follow the official style (while
keeping the old code consistent), just like all the other developers
(should) do. They make mistakes just like all other developers do. It is
unfortunate that you felt it is necessary to single out a group of
developers in a negative way (that is completely irrelevant to your
actual proposal).
> IMO; it is easier to work with the one-line style when command-line
> editing, and irrelevant when using more advanced tools.
FWIW, I do not find it easier to use one-line style when using
command-line tools. In fact, the opposite is probably true in my experience.
> As such I propose converting Squid to the same-line style:
>
> template<...>
> void foo(...)
> {
> ...
> }
>
>
> Opinions?
One-line stile increases horizontal wrapping, especially when the return
type is complex/long and there are additional markers like "static",
"inline", and "[[ noreturn ]]".
One line approach itself is inconsistent because template<> is on the
other line(s).
Searching for a function name with unknown-to-searcher or complex return
type becomes a bit more difficult when using "git log -L" and similar tools.
In many cases, function parameters should be on multiple lines as well.
Their alignment would look worse if the function name does not start the
line.
Most function definitions will have "auto" return type after we upgrade
to C++14. Whether that makes one-line style more or less attractive is
debatable, but it is an important factor in delaying related changes.
Eventually, we may be able to automatically remove explicit return types
using one of the clang-tidy tools, but such a tool does not exist yet
(modernize-use-trailing-return-type comes close but is not it).
In summary, I recommend delaying this decision until C++14. However, if
others insist on changing the format and changing it now, then I will
not block these changes, assuming newer astyle produces reasonable results.
Can new astyle support multiline formatting? If not, _that_ could be a
strong argument for changing the style.
> 2) braces on one-liner conditional blocks
>
> Current code style is a bit confused. We have it as a style to use, with
> exceptions for some old macros etc where the lack of braces causes
> compile issues or bugs.
>
> Personally I prefer the non-braced style. But it seems far safer to
> automate the always-brace style and not have those special exceptions.
>
> Opinions?
I also prefer the non-braced style for simple expressions.
Perhaps there is a way to automatically wrap complex ones, for some
reasonable definition of "complex"?
Alex.
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
[squid-dev] Coding Style updates
Hi all,
Now that we have astyle 3.1 for style enforcement we can take
advantage of it to perform a few code style change that older versions
could not.
Before I do any work testing they work I'd like to review the relevant
details of our style guidelines and see if we actually want to keep that
requirement.
1) return type on separate line from function definition.
Current style requirement:
template<...>
void
foo(...)
{
...
}
AFAIK, this based on GNU project style preferences from the far past
when Squid had a tight relationship with GNU. Today we have a conflict
between Factory code coming in with the alternative same-line style
and/or various developers being confused by the mix of code they are
touching.
IMO; it is easier to work with the one-line style when command-line
editing, and irrelevant when using more advanced tools.
As such I propose converting Squid to the same-line style:
template<...>
void foo(...)
{
...
}
Opinions?
Any reasons I missed for keeping this?
2) braces on one-liner conditional blocks
Current code style is a bit confused. We have it as a style to use, with
exceptions for some old macros etc where the lack of braces causes
compile issues or bugs.
Personally I prefer the non-braced style. But it seems far safer to
automate the always-brace style and not have those special exceptions.
Opinions?
Amos
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
