On 2/10/2014, 4:32 AM, Nicholas Nethercote wrote:
Can we just allow hand-written fixes and move forward, please?
Absolutely. I don't see any reason to enforce the usage of a tool as a
criteria for accepting a patch.
If somebody wants to contribute patches to clang-format to make it match
On Fri, Jan 31, 2014 at 6:08 AM, Benjamin Smedberg
benja...@smedbergs.us wrote:
I'm not sure that we need to fix all the problems in order to be useful. I'd
certainly automatically-generated whole-file patches which just do
re-indenting and some simple brace fixup.
Attachment
On 31/01/14 13:25, Nicholas Nethercote wrote:
I think what Bobby is saying is that a tool which restyles only lines
that have only been modified isn't much use. For example, much of
XPConnect uses 4-space indents, when it should use 2-space indents,
and fixing that cannot be sensibly done in
On Sun, Feb 2, 2014 at 5:01 PM, Anthony Jones ajo...@mozilla.com wrote:
On 31/01/14 13:25, Nicholas Nethercote wrote:
I think what Bobby is saying is that a tool which restyles only lines
that have only been modified isn't much use. For example, much of
XPConnect uses 4-space indents, when
On Sun, Feb 2, 2014 at 5:38 PM, Bobby Holley bobbyhol...@gmail.com wrote:
XPConnect currently follows JS-style, which is the most divergent style in
the tree (in particular, 4-space indents mean that a restyle is going to
rewrite every line). As such, I don't think it's a great place to
On 1/30/2014 7:25 PM, Nicholas Nethercote wrote:
One of my primary goals with starting this discussion was to reach
agreement that mass style fixes (e.g. covering entire files) are
acceptable.
I agree. We should focus our efforts on converting whole files of code
to our new style and then
On 30/01/14 10:42, Bobby Holley wrote:
On Wed, Jan 29, 2014 at 1:21 PM, Anthony Jones ajo...@mozilla.com
mailto:ajo...@mozilla.com wrote:
I don't think we should attempt style rewriting.
One thing I wanted to explain is why I have focussed on
clang-format-diff more than
On Thu, Jan 30, 2014 at 11:42 AM, Anthony Jones ajo...@mozilla.com wrote:
This tool is not going to be very helpful in a lot of modules until we
do style-rewriting. I'm not about to start taking piecemeal
2-space-indented patches in XPConnect.
When I said style rewriting I was referring to
Bobby Holley wrote:
On Tue, Jan 28, 2014 at 7:15 PM, Anthony Jones ajo...@mozilla.com wrote:
There is also some vagueness and inconsistency around:
bool
MyClass::MyFunction()
vs
bool MyClass::MyFunction()
I'm pretty sure the former is correct per Gecko style. Benjamin can confirm.
My
On 1/27/2014 7:08 PM, Nicholas Nethercote wrote:
It's been a couple of weeks now. Can we move forward?
Anthony Jones has done some work on using clang-format to restyle
lines that have changed in a file, but I don't think anyone has done
any evaluation of clang-format for whole-file changes.
On 1/28/14, 7:15 PM, Anthony Jones wrote:
On 28/01/14 13:08, Nicholas Nethercote wrote:
In the meantime, we should wrap up the pending discussions about other
changes to the style guide, such as 80/100/infinite columns,
member/parameter/local naming convention, and other threads that Gavin was
Anthony Jones writes:
There is also some vagueness and inconsistency around:
bool
MyClass::MyFunction()
vs
bool MyClass::MyFunction()
Most of the code seems to do this only for top-level functions i.e. when
not already indented. I'd like some clarification on the rule here. It
seems
On 30/01/14 06:32, Gregory Szorc wrote:
Building a style verifier for C++ is hard. You inevitably have to build
it on top of an existing parser (like Clang) or cobble something hacky
together [1], at which point you are chasing the language evolution and
all kinds of one-offs.
I wrote one
On Wed, Jan 29, 2014 at 1:21 PM, Anthony Jones ajo...@mozilla.com wrote:
I don't think we should attempt style rewriting.
One thing I wanted to explain is why I have focussed on
clang-format-diff more than clang-format. My approach is to stop
introducing ugliness so that we ratchet towards
On Wed, Jan 29, 2014 at 9:32 AM, Gregory Szorc g...@mozilla.com wrote:
On 1/28/14, 7:15 PM, Anthony Jones wrote:
On 28/01/14 13:08, Nicholas Nethercote wrote:
In the meantime, we should wrap up the pending discussions about other
changes to the style guide, such as 80/100/infinite columns,
On 2014-01-29 4:21 PM, Anthony Jones wrote:
I don't think we should attempt style rewriting.
One thing I wanted to explain is why I have focussed on
clang-format-diff more than clang-format. My approach is to stop
introducing ugliness so that we ratchet towards consistency. I suggest
we do the
On 29/01/2014 13:46, Kyle Huey wrote:
On Wed, Jan 29, 2014 at 9:32 AM, Gregory Szorc g...@mozilla.com wrote:
On 1/28/14, 7:15 PM, Anthony Jones wrote:
On 28/01/14 13:08, Nicholas Nethercote wrote:
In the meantime, we should wrap up the pending discussions about other
changes to the style
On 30/01/14 10:46, Kyle Huey wrote:
Whatever tool we end up using needs to work on Windows.
clang-format works fine on Windows.
If we want to go down the preprocessed route, you can generate
preprocessed output from the msvc compiler using:
C:\ cl /E MyFile.cpp
Pretty much does the same thing
On 30/01/14 14:44, Anthony Jones wrote:
On 30/01/14 10:46, Kyle Huey wrote:
Whatever tool we end up using needs to work on Windows.
clang-format works fine on Windows.
If we want to go down the preprocessed route, you can generate
preprocessed output from the msvc compiler using:
C:\
On Tue, Jan 7, 2014 at 2:42 AM, Patrick McManus mcma...@ducksong.com wrote:
I'm fine with enforcing a gecko wide coding style as long as it comes with
cross platform tools to act as arbiter.. it is something that needs to be
automated and isn't worth the effort of trying to get everybody on the
On 28/01/14 13:08, Nicholas Nethercote wrote:
On Tue, Jan 7, 2014 at 12:31 PM, Benjamin Smedberg
benja...@smedbergs.us wrote:
I am the owner of at least the C/C++ portions of the style guide; I propose
to wait and see whether the C++ reformatting tools are of sufficient quality
that we can
On Tue, Jan 28, 2014 at 7:15 PM, Anthony Jones ajo...@mozilla.com wrote:
In my opinion clang-format is good enough.
That's awesome!
I haven't received any feedback either positive or negative in relation
to clang-format. That makes it hard for me to know whether to continue
putting any
On Tue, Jan 7, 2014 at 12:31 PM, Benjamin Smedberg
benja...@smedbergs.us wrote:
I am the owner of at least the C/C++ portions of the style guide; I propose
to wait and see whether the C++ reformatting tools are of sufficient quality
that we can use them directly, to avoid hand-reformatting,
On 01/07/2014 01:11 AM, Joshua Cranmer wrote:
On 1/6/2014 6:06 PM, smaug wrote:
On 01/07/2014 01:38 AM, Joshua Cranmer wrote:
On 1/6/2014 4:27 PM, Robert O'Callahan wrote:
That's just not true, sorry. If some module owner decides to keep
using NULL or PRUnichar, or invent their own string
On Tuesday 2014-01-07 09:13 +0100, Ms2ger wrote:
On 01/07/2014 01:11 AM, Joshua Cranmer wrote:
Since Benjamin's message of November 22:
news://news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org
(search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you
On 1/6/2014, 8:05 PM, Karl Tomlinson wrote:
The bug is hidden almost as well by conversion of only indentation.
if (condition1)
if (condition2)
foo();
else
bar();
Sure, you could argue that style conversion makes the actual
behaviour clearer, but you'd have to know
On 1/6/2014, 8:05 PM, Karl Tomlinson wrote:
Gregory Szorc writes:
I think you just gave an example of why our tooling needs to
identify poorly formatted code better. An if without a brace
followed by multiple indented lines containing compiler
expressions is a bug at worst or
On 1/7/14 12:47 PM, Martin Thomson wrote:
I’ll just point out that this isn’t a bug if there are test cases that pass.
That depends on whether the test cases are correct (e.g. whether they
were written independently of the code, whether they were reviewed, etc)
-Boris
2014/1/7 L. David Baron dba...@dbaron.org
On Tuesday 2014-01-07 09:13 +0100, Ms2ger wrote:
On 01/07/2014 01:11 AM, Joshua Cranmer wrote:
Since Benjamin's message of November 22:
news://
news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org
(search for Please
I support getting rid of NS_ENSURE_*.
Gavin
On Tue, Jan 7, 2014 at 3:13 AM, Ms2ger ms2...@gmail.com wrote:
On 01/07/2014 01:11 AM, Joshua Cranmer wrote:
On 1/6/2014 6:06 PM, smaug wrote:
On 01/07/2014 01:38 AM, Joshua Cranmer wrote:
On 1/6/2014 4:27 PM, Robert O'Callahan wrote:
Benoit Jacob wrote:
I would like a random voice in favor of deprecating NS_ENSURE_* for the reason
that hiding control flow behind macros is IMO one of the most evil usage
patterns of macros.
So you're saying that
nsresult rv = Foo();
NS_ENSURE_SUCCESS(rv, rv);
is hiding the control flow
2014/1/7 Neil n...@parkwaycc.co.uk
Benoit Jacob wrote:
I would like a random voice in favor of deprecating NS_ENSURE_* for the
reason that hiding control flow behind macros is IMO one of the most evil
usage patterns of macros.
So you're saying that
nsresult rv = Foo();
On 1/7/2014 12:58 PM, Benoit Jacob wrote:
I would like a random voice in favor of deprecating NS_ENSURE_* for
the reason that hiding control flow behind macros is IMO one of the
most evil usage patterns of macros.
In general, I would agree. I have no problems with killing, say,
On 1/5/2014 9:34 PM, Nicholas Nethercote wrote:
I propose that we officially remove this implicit discouragement, and even
encourage changes that convert non-Mozilla-style code to Mozilla-style (with
some exceptions; see below). When modifying badly-styled code, following
existing
On 1/7/2014, 4:55 PM, Chris Peterson wrote:
On 1/7/14, 12:31 PM, Benjamin Smedberg wrote:
What's not clear to me from the discussion is whether there is a
reformatting tool which already exists which can do what we need. If
there is, I think we should actively discourage people reformatting by
On Tue, Jan 7, 2014 at 12:31 PM, Benjamin Smedberg
benja...@smedbergs.us wrote:
I am the owner of at least the C/C++ portions of the style guide; I propose
to wait and see whether the C++ reformatting tools are of sufficient quality
that we can use them directly, to avoid hand-reformatting,
Benoit Jacob wrote:
I'm scanning a function for possible early returns
Good thing you don't have to deal with C++ exceptions then.
--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
On Tue, Jan 07, 2014 at 04:13:20PM -0800, Nicholas Nethercote wrote:
On Tue, Jan 7, 2014 at 12:31 PM, Benjamin Smedberg
benja...@smedbergs.us wrote:
I am the owner of at least the C/C++ portions of the style guide; I propose
to wait and see whether the C++ reformatting tools are of
On Tue, Jan 7, 2014 at 4:38 PM, Mike Hommey m...@glandium.org wrote:
So some hand-reformatting will be inevitable.
There are actually clang-based tools for e.g. variables renaming.
Thanks for the nit. I believe the broader point -- that tools won't
fix every style issue, so some
On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote:
For example, if I'm scanning a function for possible early returns (say I'm
debugging a bug where we're forgetting to close or delete a thing before
returning), I now need to scan for NS_ENSURE_SUCCESS in addition to
On 14-01-07 08:04 PM, Kyle Huey wrote:
On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote:
For example, if I'm scanning a function for possible early returns (say I'm
debugging a bug where we're forgetting to close or delete a thing before
returning), I now need to
2014/1/7 Kyle Huey m...@kylehuey.com
On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote:
For example, if I'm scanning a function for possible early returns (say
I'm
debugging a bug where we're forgetting to close or delete a thing before
returning), I now need to
Sounds good, and I'd include also js/* so that we had consistent style
everywhere.
It is rather painful to hack various non-js/* and js/* (xpconnect in my case)
in the same patch.
(I also happen to think that Mozilla coding style is inherently better than js
style, since
it has clear rules for
A concise summary of the changes you're proposing would be useful -
here's my attempt at one.
From what I gather, the changes you're proposing to the style guide are:
* remove implicit discouragement of changing code to conform to Mozilla style
** style changes should never be combined with
I was listing only the policy changes, and would have lumped that one into
(other specifics about how this should be accomplished and with
what exceptions may need elaborating), but yes, it's certainly worth
calling out explicitly.
Gavin
On Mon, Jan 6, 2014 at 11:07 AM, Adam Roach
On 1/5/2014 8:34 PM, Nicholas Nethercote wrote:
With these ideas in mind, a goal is clear: convert non-Mozilla-style code to
Mozilla-style code, within reason.
One thing that is worth pointing out is that there are three distinct
classes of style rules, which have different implications for
I think that this is a good start, but it doesn’t go quite far enough.
Part of the problem with a policy that requires people to avoid reformatting of
stuff they don’t touch is that it propagates formatting divergence. Sometimes
because it’s better to conform to the style immediately adjacent
I think trying to change SpiderMonkey style to Gecko isn't a great use of
effort:
* There are a lot of SpiderMonkey hackers who don't hack on anything else,
and don't consider themselves Gecko hackers. Many of them don't read
dev-platform, and would probably revolt if this were to occur.
*
Hi,
two points of caution:
In the little version control archaeology I do, I hit breaks blame for
no good reason pretty often already. I'd not underestimate the cost for
the project of doing changes just for the sake of changes.
Tools don't get code right. I've seen various changes where
On 1/6/2014, 1:22 PM, Axel Hecht wrote:
Hi,
two points of caution:
In the little version control archaeology I do, I hit breaks blame for
no good reason pretty often already. I'd not underestimate the cost for
the project of doing changes just for the sake of changes.
I have very rarely run
On 01/06/2014 08:06 PM, Bobby Holley wrote:
I think trying to change SpiderMonkey style to Gecko isn't a great use of
effort:
* There are a lot of SpiderMonkey hackers who don't hack on anything else,
and don't consider themselves Gecko hackers. Many of them don't read
dev-platform, and would
On 1/6/14 12:22, Axel Hecht wrote:
In the little version control archaeology I do, I hit breaks blame
for no good reason pretty often already. I'd not underestimate the
cost for the project of doing changes just for the sake of changes.
Do you have a concrete reason to believe that Martin's
I was asked, in the previous discussion about code formatting, to
provide some research on the subject. I'm sorry I didn't get back to
that fast enough, but here we are.
We have a lot of data on this: Michael Hansen at Indiana University has
done a series of eye-tracking experiments regarding
On 1/6/14, 9:55 AM, Martin Thomson wrote:
2. create some tools
These tools should help people conform to the style.
Primarily, what is needed is a tool with appropriate configuration that runs on
the command line — e.g., mach reformat … clang-format is looking like a good
candidate for
Nicholas Nethercote writes:
We've had some recent discussions about code style. I have a propasal
I'm sceptical about the value of such changes out-weighing the
cost here. I know there will be a cost now and in the future, but
I don't recall seeing any entire files so badly formatted that it
On Sun, Jan 5, 2014 at 6:34 PM, Nicholas Nethercote
n.netherc...@gmail.com wrote:
- There is an semi-official policy that the owner of a module can dictate its
style. Examples: SpiderMonkey, Storage, MFBT.
AFAICT, there are not many rules that module owners are bound by. The
reason module
On 2014-01-06, at 12:36, Karl Tomlinson mozn...@karlt.net wrote:
I'm sceptical about the value of such changes out-weighing the
cost here. I know there will be a cost now and in the future, but
I don't recall seeing any entire files so badly formatted that it
took me more time to understand
On 2014-01-06, at 13:27, Brian Smith br...@briansmith.org wrote:
As far as PSM is concerned, my main ask is that such reformatting of
security/manager/ssl/src/* be done in February or later, so that the
current urgently-needed big refactoring in that code is not disrupted.
For one, I don’t
On Tue, Jan 7, 2014 at 7:06 AM, Bobby Holley bobbyhol...@gmail.com wrote:
* There are a lot of SpiderMonkey hackers who don't hack on anything else,
and don't consider themselves Gecko hackers.
This has always bothered me a lot. It needs to change.
Rob
--
Jtehsauts tshaei dS,o n Wohfy
On Tue, Jan 7, 2014 at 10:27 AM, Brian Smith br...@briansmith.org wrote:
AFAICT, there are not many rules that module owners are bound by.
There are lots of tree-wide or Gecko-wide rules that module owners are
bound by.
The
reason module owners can dictate style is because module owners
On 1/6/14 3:27 PM, Brian Smith wrote:
On Sun, Jan 5, 2014 at 6:34 PM, Nicholas Nethercote
n.netherc...@gmail.com wrote:
- There is an semi-official policy that the owner of a module can dictate its
style. Examples: SpiderMonkey, Storage, MFBT.
AFAICT, there are not many rules that module
Martin Thomson writes:
I would like to think that adding (or removing) braces from block statements
should be acceptable.
I would argue that braces should not be added with automation.
When debugging code, it is important to understand the intent of
the author. Adding braces at
On 01/06/2014 03:10 PM, Karl Tomlinson wrote:
Martin Thomson writes:
I would like to think that adding (or removing) braces from block statements
should be acceptable.
I would argue that braces should not be added with automation.
When debugging code, it is important to understand the
On 1/6/2014 4:27 PM, Robert O'Callahan wrote:
That's just not true, sorry. If some module owner decides to keep
using NULL or PRUnichar, or invent their own string class, they will
be corrected.
Maybe. But we also have a very large number of deprecated or
effectively-deprecated constructs
On 1/6/2014, 6:38 PM, Joshua Cranmer wrote:
On 1/6/2014 4:27 PM, Robert O'Callahan wrote:
That's just not true, sorry. If some module owner decides to keep
using NULL or PRUnichar, or invent their own string class, they will
be corrected.
Maybe. But we also have a very large number of
On 1/6/14, 3:35 PM, Daniel Holbert wrote:
On 01/06/2014 03:10 PM, Karl Tomlinson wrote:
Martin Thomson writes:
I would like to think that adding (or removing) braces from block statements
should be acceptable.
I would argue that braces should not be added with automation.
When debugging
On 01/07/2014 01:38 AM, Joshua Cranmer wrote:
On 1/6/2014 4:27 PM, Robert O'Callahan wrote:
That's just not true, sorry. If some module owner decides to keep using NULL or
PRUnichar, or invent their own string class, they will be corrected.
Maybe. But we also have a very large number of
On 01/06/2014 04:27 PM, Robert O'Callahan wrote:
That's just not true, sorry. If some module owner decides to keep using
NULL or PRUnichar, or invent their own string class, they will be corrected.
At least the former isn't true, as far as I remember. It took convincing to
get JS to move over
On 1/6/2014 6:06 PM, smaug wrote:
On 01/07/2014 01:38 AM, Joshua Cranmer wrote:
On 1/6/2014 4:27 PM, Robert O'Callahan wrote:
That's just not true, sorry. If some module owner decides to keep
using NULL or PRUnichar, or invent their own string class, they will
be corrected.
Maybe. But we
On 01/06/2014 04:46 PM, Jason Orendorff wrote:
I'm fine with changing SpiderMonkey in whatever way is necessary to stop
these threads forever.
Pretty sure I've said the same thing in other places, that we should have
something everyone hates and use it everywhere. Definitely I've said it to
I'm fine with enforcing a gecko wide coding style as long as it comes with
cross platform tools to act as arbiter.. it is something that needs to be
automated and isn't worth the effort of trying to get everybody on the same
page by best effort.
On Mon, Jan 6, 2014 at 5:41 PM, Ehsan Akhgari
Gregory Szorc writes:
I think you just gave an example of why our tooling needs to
identify poorly formatted code better. An if without a brace
followed by multiple indented lines containing compiler
expressions is a bug at worst or unreadable/unclear code at
best. We could, of course,
On Jan 6, 2014, at 17:10, Karl Tomlinson mozn...@karlt.net wrote:
Gregory Szorc writes:
I think you just gave an example of why our tooling needs to
identify poorly formatted code better. An if without a brace
followed by multiple indented lines containing compiler
expressions is a bug at
On 1/6/14, 5:05 PM, Karl Tomlinson wrote:
If we have a tool to skip the style change on any such unclear
situations, then perhaps we can proceed more safely.
I would replace skip with abort loudly, so a human can review the
unclear code. :)
chris
We've had some recent discussions about code style. I have a propasal
For the purpose of this proposal I will assume that there is consensus on the
following ideas.
- Having multiple code styles is bad.
- Therefore, reducing the number of code styles in our code is a win (though
there are
75 matches
Mail list logo
