Quick update.

It looks like clang-format is actually okay with C-style comments being on
their own line before a method definition. So last night patches landed to move
all these comments to their own line.

From:
/* static */ void Foo::Bar() { ... }

To:
/* static */
void Foo::Bar() { ... }

So why did the initial reformatting move all these comments around?

I believe the issue is that before the reformat, the C-style comments were on
the same line as the return types of the methods [1]. I think this lead
clang-format to view the comments as being part of the method definitions, and
so they were all formatted to be on the same line.

[1] 
https://searchfox.org/mozilla-central/diff/265e6721798a455604328ed5262f430cfcc37c2f/layout/base/nsLayoutUtils.cpp#1098

Thanks,
Ryan

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, January 30, 2019 10:17 AM, Ryan Hunt <rh...@eqrion.net> wrote:

> I've filed bug 1523969 to consider making this change.
>
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1523969)
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Monday, January 28, 2019 6:27 PM, Ryan Hunt <rh...@eqrion.net> wrote:
>
>> Yeah, personally I have found them be useful and don't have an issue with 
>> keeping
>> them. I just wasn't sure if that was a common experience.
>>
>> So for converting from C-style to C++-style, that would be:
>>
>> /* static */ void Foo::Bar() {
>>  ...
>> }
>>
>> // static
>> void Foo::Bar() {
>>  ...
>> }
>>
>> I think that would be good. My one concern would be the presence of other 
>> C++-style
>> comments before the method definition. For example [1].
>>
>> Ideally documentation like that should go in the header by the method 
>> declaration, but I
>> have no idea if we consistently do that.
>>
>> [1] 
>> https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1023
>>
>> Thanks,
>> Ryan
>>
>> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
>> On Monday, January 28, 2019 12:51 PM, Ehsan Akhgari 
>> <ehsan.akhg...@gmail.com> wrote:
>>
>>> This is indeed one of the cases where the reformat has made things worse.  
>>> I think as a couple of people have already said, we'll find that some 
>>> people do find these annotations useful, even if they're not always 
>>> consistently present.
>>>
>>> The path to least resistance for addressing this problem may be to convert 
>>> these into C++-style comments and therefore moving them into their own 
>>> lines.  Would you be OK with that?
>>>
>>> Thanks,
>>> Ehsan
>>>
>>> On Fri, Jan 25, 2019 at 11:49 PM Ryan Hunt <rh...@eqrion.net> wrote:
>>>
>>>> Hi all,
>>>>
>>>> Quick C++ style question.
>>>>
>>>> A common pattern in Gecko is for method definitions to have a comment with 
>>>> the
>>>> 'static' or 'virtual' qualification.
>>>>
>>>> Before the reformat, the comment would be on it's own separate line [1]. 
>>>> Now
>>>> it's on the main line of the definition [2].
>>>>
>>>> For example:
>>>>
>>>> /* static */ void
>>>> Foo::Bar() {
>>>>   ...
>>>> }
>>>>
>>>> vs.
>>>>
>>>> /* static */ void Foo::Bar() {
>>>>   ...
>>>> }
>>>>
>>>> Personally I think this now takes too much horizontal space from the main
>>>> definition, and would prefer it to be either on its own line or just 
>>>> removed.
>>>>
>>>> Does anyone have an opinion on whether we still want these comments? And 
>>>> if so
>>>> whether it makes sense to move them back to their own line.
>>>>
>>>> (My ulterior motive is that sublime text's indexer started failing to find
>>>>  these definitions after the reformat, but that should be fixed regardless)
>>>>
>>>> If you're interested in what removing these would entail, I wrote a regex 
>>>> to
>>>> make the change [3].
>>>>
>>>> Thanks,
>>>> Ryan
>>>>
>>>> [1] 
>>>> https://hg.mozilla.org/mozilla-central/file/0348d472115d/layout/generic/nsFrame.cpp#l1759
>>>> [2] 
>>>> https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1756
>>>> [3] https://hg.mozilla.org/try/rev/31ab3e466b6f15dcdbb1aee47edabc7c358b86f2
>>>>
>>>> _______________________________________________
>>>> dev-platform mailing list
>>>> dev-platform@lists.mozilla.org
>>>> https://lists.mozilla.org/listinfo/dev-platform
>>>
>>> --
>>> Ehsan
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to