-
Notifications
You must be signed in to change notification settings - Fork 271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Style Guide Corrections ;) #2
Comments
Jeremy, This is excellent feedback and is much appreciated. Thank you!
Changed here: 233ea6a All of the issues with this guide's treatment of comments have been moved to #6.
Note added here: 05ad9c6
Moving this to #3 for further discussion.
I agree that the focus should be on readability, not following a rule for the rule's sake. An attempt at communicating this (using your examples): d3ccb1f
Agreed:
This alignment can be visually pleasing in certain scenarios, but the drawbacks also need to be considered (such as the increased maintenance burden). I've included a few examples below, directly from the docco source. (My intention isn't to pick on docco specifically, but rather to demonstrate how hard it is to consistently apply this alignment rule.) This looks nice: fs = require 'fs'
path = require 'path'
showdown = require('./../vendor/showdown').Showdown Including the next line: fs = require 'fs'
path = require 'path'
showdown = require('./../vendor/showdown').Showdown
{spawn, exec} = require 'child_process' Is the inconsistency deliberate? Later on: # Create the template that we will use to generate the Docco HTML page.
docco_template = template fs.readFileSync(__dirname + '/../resources/docco.jst').toString()
# The CSS styles we'd like to apply to the documentation.
docco_styles = fs.readFileSync(__dirname + '/../resources/docco.css').toString() This is consistent alignment, but I would argue that it looks somewhat awkward with the comments interspersed. (The statements look better when viewing the output that docco generates when run against its own source. Which reading mode is more important to optimize for?) A few lines down and there is another example of an inconsistency: # The end of each Pygments highlight block.
highlight_end = '</pre></div>'
# Run the script.
# For each source file passed in as an argument, generate the documentation.
sources = process.ARGV.sort() Overall it seems unrealistic to mandate this sort of alignment across-the-board. A reasonable compromise could read something like: "In general, do not align left and right values in assignment statements, unless an uninterrupted sequence of assignment statements follow each other." With this rule, the following code (also taken exactly from docco) is compliant: parse = (source, code) ->
lines = code.split '\n'
sections = []
language = get_language source
has_code = docs_text = code_text = ''
save = (docs, code) ->
sections.push docs_text: docs, code_text: code And an earlier example could validly look like this: fs = require 'fs'
path = require 'path'
showdown = require('./../vendor/showdown').Showdown
{spawn, exec} = require 'child_process' Or like this: fs = require 'fs'
path = require 'path'
showdown = require('./../vendor/showdown').Showdown
{spawn, exec} = require 'child_process' I'm not sure that I'm sold on this rule, but it's worth mulling over further. Thanks, |
... I dig it. That's a nice rule. And even then, it's an opt-in thing -- you don't have to do it. |
I find that omitting the I find this
In the first case the
|
Why is the standalone @ not ok? Overall, I like consistency unless there's a functional disadvantage. And in this case it's also usability: search only for @, not both @ and this |
@andreineculau it looks shitty |
Do you know, why @ is used for "this."? One can make "this" from characters of "shit". shitChars = 'shit'.split('')
thisChars = [shitChars.pop(), shitChars.splice(1,1)[0], shitChars.pop(), shitChars.pop()]
console.assert thisChars.join('') is 'this', 'word seems to be improved'
console.assert shitChars.length is 0, 'no pieces of old "shit" are left' Now seriously - I think, if we start using "@" (and we should) anywhere in the code, we shall keep it consistent for standalone use too. |
Instead of doing this in a fork -- I'd rather put these in a ticket to make conversation easier...
... I think this particular point is still fairly undecided. There are a lot of sequences of CoffeeScript where aligning your left and right values to look more like a table is pleasing. I know a style guide has to pick one way or the other, but I think it's still an open question.
Nope -- quite the opposite. If
a = b
should always be written with spaces around the=
, then the same holds true in default arguments, Python notwithstanding.Actually, CoffeeScript tries to encourage pervasive commenting ... not so much to answer the "what" or the "how" of any individual line of code, but to answer the "why".
Nope -- comments should be written as full sentences, with proper punctuation.
The bit that you describe as "block comments" aren't CoffeeScript block comments, which look like this:
... but, the ones that you describe are always preferred instead of block comments -- which should only be used if you intend to pass along the comment to the underlying JS, like a license, or warning.
Nope -- poor comments are poor, and good comments are good. There's no problem with writing good inline comments to explain an un-obvious function or tricky algorithm.
I don't think that this one is settled quite yet. In JavaScript, of course, until
const
lands, there's no such thing as a constant. For that reason, I think that simply using local variable style for constants may actually make more sense. I'm not sure which should be prescribed.I think that parentheses inclusion or omission doesn't have a hard and fast rule. For example, your second example doesn't benefit:
Usually, it should be obvious (or at least obvious-ish) when leaving parentheses (and braces) off makes things clearer, and when it doesn't:
I personally don't use it, or recommend it, but I would think that folks that do prefer it would take issue with this rule.
Finally, you may want to add a note to always avoid use of standalone
@
, all by itself. A simplethis
is preferred. (A common question.)Thanks for this style guide -- it's really great stuff.
The text was updated successfully, but these errors were encountered: