Labs/Jetpack/Reboot/Style Guide: Difference between revisions
(Added Proposal to document options objects) |
(Went to town) |
||
| Line 1: | Line 1: | ||
Code style bikeshedding is great, let's do it, here we go. | Code style bikeshedding is great, let's do it, here we go. | ||
== Initial Discussion == | |||
From {{bug|541622}}: | |||
<pre> | <pre> | ||
| Line 77: | Line 81: | ||
Note: in Mozilla code, placing the dot "member operator" at the end of the first line of a member operation that spans two lines is the newer convention that became popular during early development of Firefox, while placing it at the beginning of the next line is the older convention that was popular in Mozilla Suite code (and I think is still the convention in Seamonkey and possibly Thunderbird). | Note: in Mozilla code, placing the dot "member operator" at the end of the first line of a member operation that spans two lines is the newer convention that became popular during early development of Firefox, while placing it at the beginning of the next line is the older convention that was popular in Mozilla Suite code (and I think is still the convention in Seamonkey and possibly Thunderbird). | ||
---- | == Proposals == | ||
'''These are just proposals! Don't like? Say so, let's git'r done.''' | |||
=== Style === | |||
If all branches of a conditional are single-liners, no braces needed: | |||
<pre class="brush:js;toolbar:false;"> | |||
if (foo) | |||
bar(); | |||
else | |||
baz(); | |||
</pre> | |||
If a branch requires more than one line so as not to exceed 80 columns, use braces: | |||
<pre class="brush:js;toolbar:false;"> | |||
if (foo) { | |||
Cc["@mozilla.org/appshell/window-mediator;1"]. | |||
getService(Ci.nsIWindowMediator). | |||
getEnumerator("navigator:browser"). | |||
doSomethingOnce(); | |||
} | |||
</pre> | |||
If any branch requires braces, use them for all: | |||
<pre class="brush:js;toolbar:false;"> | |||
if (foo) { | |||
Cc["@mozilla.org/appshell/window-mediator;1"]. | |||
getService(Ci.nsIWindowMediator). | |||
getEnumerator("navigator:browser"). | |||
doSomethingOnce(); | |||
} | |||
else { | |||
baz(); | |||
} | |||
</pre> | |||
Loops always require braces: | |||
<pre class="brush:js;toolbar:false;"> | |||
for (let i = 0; i < len; i++) { | |||
arr[i] = 0; | |||
} | |||
</pre> | |||
Do not cuddle <tt>else</tt> and <tt>catch</tt>. | |||
<pre class="brush:js;toolbar:false;"> | |||
// Do this: | |||
if (foo) { | |||
bar(); | |||
baz(); | |||
} | |||
else { | |||
qux(); | |||
} | |||
// Not this: | |||
if (foo) { | |||
bar(); | |||
baz(); | |||
} else { | |||
qux(); | |||
} | |||
</pre> | |||
Dots at the end of lines, not the front: | |||
<pre class="brush:js;toolbar:false;"> | |||
// Do this: | |||
Cc["@mozilla.org/appshell/window-mediator;1"]. | |||
getService(Ci.nsIWindowMediator). | |||
getEnumerator("navigator:browser"). | |||
doSomethingOnce(); | |||
// Not this: | |||
Cc["@mozilla.org/appshell/window-mediator;1"] | |||
.getService(Ci.nsIWindowMediator) | |||
.getEnumerator("navigator:browser") | |||
.doSomethingOnce(); | |||
</pre> | |||
=== Comments and Documentation === | |||
This applies to in-source docs only, not nice docs meant for end users. | |||
Javadoc-style comments for functions (and maybe classes?). Javadocs are conventional throughout Mozilla code. | |||
Other comments are C++-style. | |||
If a comment is a full sentence, capitalize and punctuate it. Full sentences are preferable to fragments, but fragments are sometimes more effective. Don't capitalize or punctuate fragments. | |||
Documenting function params that are options objects: | |||
<pre class="brush:js;toolbar:false;"> | <pre class="brush:js;toolbar:false;"> | ||
Revision as of 01:56, 26 January 2010
Code style bikeshedding is great, let's do it, here we go.
Initial Discussion
From bug 541622:
> Looks like we are using javadoc/jsdoc style comments to document functions.
Oh, meant to mention that. I'm more concerned about documenting functions in the first place than the style we use to do it. Javadoc is definitely widely used in Mozilla for both C++ and JS, so considering that one day other people will be maintaining this code, I think it's a good idea to use it.
> I wonder if it'd be helpful to just go ahead and add the function name > to this comment, as it'd make it easier to read and for jsdoc-esque-tools > to parse.
Personally I don't like duping the name in the comment for a couple of reasons: It's more verbose, requiring me to read more (and Javadocs are verbose enough), and there's a danger of the name in the comment and the real name going out of sync. Javadocs elsewhere in Mozilla don't do it FWIW.
> Below, I am used to the catch being on the same line as the previous > closing bracket before it. Is that weird?
Gross. Actually, people use both styles. I like new lines, and the JS style guide prefers new lines for elses and I presume catches too. We can bikeshed, but we should decide if we want to enforce a style and if so do.
> Is there any way to format javadoc/jsdoc text as being in monospaced > font? Can we borrow from Markdown here or anything? In particular when > you mention 'str' below I wasn't sure if you were referring to the > word "string" or a variable called "str".
Javadocs are HTML, so <tt>foo</tt> or such would be technically correct, but in Mozilla I don't think there's generally any expectation that comments will be viewed outside of the source. I've seen |foo|, 'foo', "foo".
> Some code style guides strictly specify that developers should only use one > form > of commenting, C-style /* */ or C++-style //. The MDC Coding Style page doesn't > seem to make any distinction here; what do we want to do?
I only use C-style for Javadocs of functions. C++ style for everything else. That's consistent with Mozilla in general.
> Interesting that you put the dot at the end of the same line as the > CC[], rather than at the beginning of the call to createInstance(). > I've seen both on MDC snippets and would like to standardize on one > or the other. > > >+ var stream = Cc['@mozilla.org/network/file-output-stream;1']. > >+ createInstance(Ci.nsIFileOutputStream);
Yeah. I picked it up from Crockford, the danger being that since JS inserts semicolons automatically there's some amount of danger in prefixing the dot, e.g., if you accidentally make a new line where there shouldn't be. Leaving the dot at the end also lets my eyes read less. I'd like to use indentation to indicate continuation, but I'm not always consistent, e.g.:
let winEnum = Cc["@mozilla.org/appshell/window-mediator;1"].
getService(Ci.nsIWindowMediator).
getEnumerator("navigator:browser");
Myk: IMHO, when the first character of the expression is already significantly indented from the first character of the line, indentation of subsequent lines of the expression to the same column as the first character is plenty of indentation and has the beneficial side-effect of creating a strong vertical alignment down the left hand side of the expression, which makes it easier to perceive the connectedness of its elements, i.e.:
let winEnum = Cc["@mozilla.org/appshell/window-mediator;1"].
getService(Ci.nsIWindowMediator).
getEnumerator("navigator:browser");
However, in cases where you only need to use the result once and can throw it away afterwards, and thus you don't need to assign it to a variable, it makes sense to indent to clarify that the expression spans lines, i.e.:
Cc["@mozilla.org/appshell/window-mediator;1"].
getService(Ci.nsIWindowMediator).
getEnumerator("navigator:browser").
doSomethingOnce();
Note: in Mozilla code, placing the dot "member operator" at the end of the first line of a member operation that spans two lines is the newer convention that became popular during early development of Firefox, while placing it at the beginning of the next line is the older convention that was popular in Mozilla Suite code (and I think is still the convention in Seamonkey and possibly Thunderbird).
Proposals
These are just proposals! Don't like? Say so, let's git'r done.
Style
If all branches of a conditional are single-liners, no braces needed:
if (foo) bar(); else baz();
If a branch requires more than one line so as not to exceed 80 columns, use braces:
if (foo) {
Cc["@mozilla.org/appshell/window-mediator;1"].
getService(Ci.nsIWindowMediator).
getEnumerator("navigator:browser").
doSomethingOnce();
}
If any branch requires braces, use them for all:
if (foo) {
Cc["@mozilla.org/appshell/window-mediator;1"].
getService(Ci.nsIWindowMediator).
getEnumerator("navigator:browser").
doSomethingOnce();
}
else {
baz();
}
Loops always require braces:
for (let i = 0; i < len; i++) {
arr[i] = 0;
}
Do not cuddle else and catch.
// Do this:
if (foo) {
bar();
baz();
}
else {
qux();
}
// Not this:
if (foo) {
bar();
baz();
} else {
qux();
}
Dots at the end of lines, not the front:
// Do this:
Cc["@mozilla.org/appshell/window-mediator;1"].
getService(Ci.nsIWindowMediator).
getEnumerator("navigator:browser").
doSomethingOnce();
// Not this:
Cc["@mozilla.org/appshell/window-mediator;1"]
.getService(Ci.nsIWindowMediator)
.getEnumerator("navigator:browser")
.doSomethingOnce();
Comments and Documentation
This applies to in-source docs only, not nice docs meant for end users.
Javadoc-style comments for functions (and maybe classes?). Javadocs are conventional throughout Mozilla code.
Other comments are C++-style.
If a comment is a full sentence, capitalize and punctuate it. Full sentences are preferable to fragments, but fragments are sometimes more effective. Don't capitalize or punctuate fragments.
Documenting function params that are options objects:
/**
* This function takes an options dictionary.
*
* @param options
* An options dictionary. It may define the following properties:
* @prop foo
* If true, does the thing with the foo.
* @prop bar
* Don't use this one. Ever.
*/
function (options) {}