Re: C++ method definition comments
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 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 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
>> 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 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
Re: C++ method definition comments
On 1/30/19 10:19 AM, Ehsan Akhgari wrote: What tool do you use which has difficulty showing function names in diffs right now? It seems to work fine for me both in git and hgweb... "hg diff" and "git diff" both fail at this over here when there is a /* comment */ before the function name. hg version 4.4.2 and git version 2.14.5. For example, if you make a change in the middle of nsContentUtils::AttemptLargeAllocationLoad and then hg diff or git diff, it will claim your change is in nsContentUtils::LookupCustomElementDefinition. I just ran into this while trying to read a patch someone asked for review on. If there is no comment before the function name, it works. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
Ehsan Akhgari writes: > What tool do you use which has difficulty showing function names in diffs > right now? It seems to work fine for me both in git and hgweb... It's cases like these that are truncated earlier due to putting the return type before the function name: % hg export 9f7b93f5c4f8 | sed -n 275p @@ -891,19 +886,17 @@ nsresult nsMixedContentBlocker::ShouldLo % hg export b8d2dfdfc324 | sed -n 2094p @@ -1717,16 +1860,23 @@ already_AddRefed nsFactoryEn https://hg.mozilla.org/mozilla-central/rev/9f7b93f5c4f8#l5.119 https://hg.mozilla.org/mozilla-central/rev/b8d2dfdfc324#l7.838 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
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 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
> 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 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
Re: C++ method definition comments
On Wed, Jan 30, 2019 at 1:35 AM Karl Tomlinson wrote:
> Ehsan Akhgari writes:
>
> > On Mon, Jan 28, 2019 at 2:58 PM Jeff Gilbert
> wrote:
> >
> >> I would much rather revert to:
> >> /*static*/ void
> >> Foo::Bar()
> >>
> >> The Foo::Bar is the most relevant part of that whole expression, which
> >> makes it nice to keep up against the start of the line.
> >>
> >
> > The clang-format option which allows formatting the way you are
> suggesting,
> > AlwaysBreakAfterDefinitionReturnType, is deprecated, and is likely to be
> > removed from a future version of clang-format, so there is no sustainable
> > way for us to adopt this suggestion.
>
> Where there's a will there's often a way. e.g.
>
> /*static*/ void //(clang-format line-break)
> Foo::Bar() {
>
> I do like being able to see function names in diff output, but I'm
> not so keen on having to put //cflb at the beginning of every
> function. This feels too much like working against the decision
> to follow Google style.
>
> With so much code using Google style, I guess there must be tools
> to show useful information in diff output, or at least there will
> be soon...
>
What tool do you use which has difficulty showing function names in diffs
right now? It seems to work fine for me both in git and hgweb...
--
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
On Wed, Jan 30, 2019 at 1:30 AM Karl Tomlinson wrote:
> Ehsan Akhgari writes:
>
> > On Mon, Jan 28, 2019 at 6:27 PM Ryan Hunt wrote:
> >
> >> [...]
> >>
> >> So for converting from C-style to C++-style, that would be:
> >>
> >> /* static */ void Foo::Bar() {
> >> ...
> >> }
> >>
> >> // static
> >> void Foo::Bar() {
> >> ...
> >> }
> >>
> >> [...]
> >>
> >> My one concern would be the presence of other C++-style
> >> comments before the method definition. For example [1].
> >
> > [...] How about detecting those cases and inserting a newline
> > between the comments on the line before, for extra clarity?
> >
> >> [...]
> >>
> >> [1]
> >>
> https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1023
> >>
> >> ‐‐‐ Original Message ‐‐‐
> >> On Monday, January 28, 2019 12:51 PM, Ehsan Akhgari <
> >> ehsan.akhg...@gmail.com> wrote:
> >>
> >> [...]
> >>
> >> 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?
>
> I haven't noticed clang-format enforcing its own opinions on
> comments when they already follow Google style.
>
> In my experiments clang-format is accepting this:
>
> // Make this Foo Bar.
> /* static */
> void Foo::Bar() {
> ...
> }
>
> The /* */ style comment provides a clear separation from any other
> comment on the previous line, without the need for an extra
> blank-line. "don't use blank lines when you don't have to."
>
It depends on where you start from. If you start from the code sample
above, clang-format won't touch the lines with comments. However if you
start from the code sample below, it will:
// Make this Foo Bar.
/* static */ void
Foo::Bar()
{
// ...
}
It will get reformatted to:
// Make this Foo Bar.
/* static */ void Foo::Bar() {
// ...
}
Cheers,
--
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
On Tue, Jan 29, 2019 at 6:40 PM wrote:
> On Wednesday, January 30, 2019 at 9:57:02 AM UTC+11, Ehsan Akhgari wrote:
> > On Mon, Jan 28, 2019 at 7:10 PM wrote:
> >
> > > Just a thought: Would it be worth considering a blank macro, e.g.:
> > > static void foo();
> > > DECLARED_STATIC void foo() {...}
> > >
> > > On top of not being confused with other comments around, it could be
> > > clang-checked so it's never wrong. (And maybe eventually enforced, like
> > > MOZ_IMPLICIT is.)
> > >
> >
> > Yeah, that could be a future alternative, but it would require someone to
> > do the hard work of implementing the required static analysis and landing
> > it. I think Ryan's proposal will probably simplify that process somewhat
> > by making it possible to mass-replace a bunch of "// static" comments
> with
> > "DECLARED_STATIC" or some such, but I don't want to hold the good
> solution
> > here for a perfect solution in the future. I would personally be OK for
> > these two to happen incrementally.
> >
> > Would you mind filing a bug for this please?
> >
> > Thanks,
> > Ehsan
>
> Thank you Ehsan. Yes, a phased approach would definitely be the way to go.
> https://bugzilla.mozilla.org/show_bug.cgi?id=1523793
>
> I realize now that this doesn't help at all with the issue of valuable
> horizontal real-estate! Sorry for the tangent.
>
Oh but I think it will actually. :-) clang-format has a heuristic that
makes it put upper-case macros before function definition return types on
their own lines (e.g. see how our NS_IMETHODIMP macro is treated.)
> One (partly tongue-in-cheek) solution to make the important function name
> more prominent is to use trailing return types:
> `auto Foo::Bar() -> void {`
> Note that this is more easily greppable when searching for function names.
> And it looks more like Rust.
>
> To help with spacing, the `DECLARED_...` macros could be placed at the end
> (if possible?) :
> `void Foo::Bar() DECLARED_STATIC {`
> `auto Foo::Bar() -> void DECLARED_STATIC {`
> But this feels uglier to me.
>
Interesting idea! I wonder if we are at a point in our adoption curve of
Rust that we should worry about how weird our C++ code looks for the
majority of developers yet... I suppose doing this would mean reserving
only five characters at the beginning of function names for the "return
type".
> Cheers,
> Gerald
>
> > > On Tuesday, January 29, 2019 at 10:27:17 AM UTC+11, Ryan Hunt 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...@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
> 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
Re: C++ method definition comments
Ehsan Akhgari writes:
> On Mon, Jan 28, 2019 at 2:58 PM Jeff Gilbert wrote:
>
>> I would much rather revert to:
>> /*static*/ void
>> Foo::Bar()
>>
>> The Foo::Bar is the most relevant part of that whole expression, which
>> makes it nice to keep up against the start of the line.
>>
>
> The clang-format option which allows formatting the way you are suggesting,
> AlwaysBreakAfterDefinitionReturnType, is deprecated, and is likely to be
> removed from a future version of clang-format, so there is no sustainable
> way for us to adopt this suggestion.
Where there's a will there's often a way. e.g.
/*static*/ void //(clang-format line-break)
Foo::Bar() {
I do like being able to see function names in diff output, but I'm
not so keen on having to put //cflb at the beginning of every
function. This feels too much like working against the decision
to follow Google style.
With so much code using Google style, I guess there must be tools
to show useful information in diff output, or at least there will
be soon...
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
Ehsan Akhgari writes:
> On Mon, Jan 28, 2019 at 6:27 PM Ryan Hunt wrote:
>
>> [...]
>>
>> So for converting from C-style to C++-style, that would be:
>>
>> /* static */ void Foo::Bar() {
>> ...
>> }
>>
>> // static
>> void Foo::Bar() {
>> ...
>> }
>>
>> [...]
>>
>> My one concern would be the presence of other C++-style
>> comments before the method definition. For example [1].
>
> [...] How about detecting those cases and inserting a newline
> between the comments on the line before, for extra clarity?
>
>> [...]
>>
>> [1]
>> https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1023
>>
>> ‐‐‐ Original Message ‐‐‐
>> On Monday, January 28, 2019 12:51 PM, Ehsan Akhgari <
>> ehsan.akhg...@gmail.com> wrote:
>>
>> [...]
>>
>> 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?
I haven't noticed clang-format enforcing its own opinions on
comments when they already follow Google style.
In my experiments clang-format is accepting this:
// Make this Foo Bar.
/* static */
void Foo::Bar() {
...
}
The /* */ style comment provides a clear separation from any other
comment on the previous line, without the need for an extra
blank-line. "don't use blank lines when you don't have to."
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
On Wednesday, January 30, 2019 at 9:57:02 AM UTC+11, Ehsan Akhgari wrote:
> On Mon, Jan 28, 2019 at 7:10 PM wrote:
>
> > Just a thought: Would it be worth considering a blank macro, e.g.:
> > static void foo();
> > DECLARED_STATIC void foo() {...}
> >
> > On top of not being confused with other comments around, it could be
> > clang-checked so it's never wrong. (And maybe eventually enforced, like
> > MOZ_IMPLICIT is.)
> >
>
> Yeah, that could be a future alternative, but it would require someone to
> do the hard work of implementing the required static analysis and landing
> it. I think Ryan's proposal will probably simplify that process somewhat
> by making it possible to mass-replace a bunch of "// static" comments with
> "DECLARED_STATIC" or some such, but I don't want to hold the good solution
> here for a perfect solution in the future. I would personally be OK for
> these two to happen incrementally.
>
> Would you mind filing a bug for this please?
>
> Thanks,
> Ehsan
Thank you Ehsan. Yes, a phased approach would definitely be the way to go.
https://bugzilla.mozilla.org/show_bug.cgi?id=1523793
I realize now that this doesn't help at all with the issue of valuable
horizontal real-estate! Sorry for the tangent.
One (partly tongue-in-cheek) solution to make the important function name more
prominent is to use trailing return types:
`auto Foo::Bar() -> void {`
Note that this is more easily greppable when searching for function names.
And it looks more like Rust.
To help with spacing, the `DECLARED_...` macros could be placed at the end (if
possible?) :
`void Foo::Bar() DECLARED_STATIC {`
`auto Foo::Bar() -> void DECLARED_STATIC {`
But this feels uglier to me.
Cheers,
Gerald
> > On Tuesday, January 29, 2019 at 10:27:17 AM UTC+11, Ryan Hunt 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
> > 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 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
> > > >>
> > > >
> > > > --
> > > > Ehsan
> >
>
>
> --
> Ehsan
Re: C++ method definition comments
On Mon, Jan 28, 2019 at 7:10 PM wrote:
> Just a thought: Would it be worth considering a blank macro, e.g.:
> static void foo();
> DECLARED_STATIC void foo() {...}
>
> On top of not being confused with other comments around, it could be
> clang-checked so it's never wrong. (And maybe eventually enforced, like
> MOZ_IMPLICIT is.)
>
Yeah, that could be a future alternative, but it would require someone to
do the hard work of implementing the required static analysis and landing
it. I think Ryan's proposal will probably simplify that process somewhat
by making it possible to mass-replace a bunch of "// static" comments with
"DECLARED_STATIC" or some such, but I don't want to hold the good solution
here for a perfect solution in the future. I would personally be OK for
these two to happen incrementally.
Would you mind filing a bug for this please?
Thanks,
Ehsan
> Cheers,
> Gerald
>
> On Tuesday, January 29, 2019 at 10:27:17 AM UTC+11, Ryan Hunt 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
> 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 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
> > >>
> > >
> > > --
> > > Ehsan
> ___
> 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
Re: C++ method definition comments
On Mon, Jan 28, 2019 at 2:58 PM Jeff Gilbert wrote:
> I would much rather revert to:
> /*static*/ void
> Foo::Bar()
>
> The Foo::Bar is the most relevant part of that whole expression, which
> makes it nice to keep up against the start of the line.
>
The clang-format option which allows formatting the way you are suggesting,
AlwaysBreakAfterDefinitionReturnType, is deprecated, and is likely to be
removed from a future version of clang-format, so there is no sustainable
way for us to adopt this suggestion.
> In a clang-format world, we should feel more free to make such
> deviations from Google Style, since it's all handled for us.
>
> On Mon, Jan 28, 2019 at 10:52 AM Ehsan Akhgari
> 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 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
>
--
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
On Mon, Jan 28, 2019 at 6:27 PM Ryan Hunt 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.
>
Great!
> My one concern would be the presence of other C++-style
> comments before the method definition. For example [1].
>
That's nothing that a bit of regex wizardry can't take care of. :-) How
about detecting those cases and inserting a newline between the comments on
the line before, for extra clarity?
> 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 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
>
>
>
--
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
Just a thought: Would it be worth considering a blank macro, e.g.:
static void foo();
DECLARED_STATIC void foo() {...}
On top of not being confused with other comments around, it could be
clang-checked so it's never wrong. (And maybe eventually enforced, like
MOZ_IMPLICIT is.)
Cheers,
Gerald
On Tuesday, January 29, 2019 at 10:27:17 AM UTC+11, Ryan Hunt 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
> 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 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
> >>
> >
> > --
> > Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
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
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 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
Re: C++ method definition comments
I would much rather revert to:
/*static*/ void
Foo::Bar()
The Foo::Bar is the most relevant part of that whole expression, which
makes it nice to keep up against the start of the line.
In a clang-format world, we should feel more free to make such
deviations from Google Style, since it's all handled for us.
On Mon, Jan 28, 2019 at 10:52 AM Ehsan Akhgari 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 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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
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 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
Re: C++ method definition comments
I find them extremely useful, too (as in "removing them would make my life miserable in quite a few bugs"). I have no problem with putting them on a separate line. Cheers, David On 26/01/2019 15:19, Jonathan Watt wrote: > Personally I find them useful. Putting them on a separate line seems > reasonable > to me. > > Jonathan > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: C++ method definition comments
Personally I find them useful. Putting them on a separate line seems reasonable
to me.
Jonathan
On 26/01/2019 04:49, Ryan Hunt 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
C++ method definition comments
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
