|
| 1 | +Contributing |
| 2 | +============ |
| 3 | + |
| 4 | +Thank you for contributing to vimperator-labs! |
| 5 | + |
| 6 | +Following are some guidelines designed to make the progress as smooth as |
| 7 | +possible. If you have any questions, feel free to drop by |
| 8 | +`#vimperator@freenode` or create an issue. |
| 9 | + |
| 10 | +Issues |
| 11 | +------ |
| 12 | + |
| 13 | +A few things to keep in mind when creating a new issue: |
| 14 | + |
| 15 | +- Follow the issue template |
| 16 | +- Attach relevant configuration or RC file if applicable (e.g. your |
| 17 | + `.vimperatorrc` or part of it). |
| 18 | +- Check if a fresh profile solves the issue |
| 19 | + - `$ firefox -no-remote -P` |
| 20 | +- Confirm if it works without your configuration |
| 21 | + - `$ firefox -no-remote -P <fresh profile> -vimperator "+u NONE"` |
| 22 | + |
| 23 | +Pull requests |
| 24 | +------------- |
| 25 | + |
| 26 | +- Title and commit message(s) should include relevant issue ID(s) |
| 27 | +- For any new or changed feature, AsciiDoc documentation and an entry in the |
| 28 | + NEWS file is required for the patch to be accepted. |
| 29 | + |
| 30 | +Development installation |
| 31 | +------------------------ |
| 32 | + |
| 33 | +Creating and installing a new XPI file after each update is cumbersome. |
| 34 | +Instead, create an extension linking to the vimperator directory, for example |
| 35 | +of a Git clone. |
| 36 | + |
| 37 | +1. Find the location your [Firefox profile](http://kb.mozillazine.org/Profile_folder_-_Firefox) |
| 38 | + - E.g. `/home/user/.mozilla/firefox/<hash>.default` |
| 39 | +2. Go into the `extensions` directory |
| 40 | + |
| 41 | +4. Create a `[email protected]` text file |
| 42 | + - Add the absolute path to your Vimperator directory |
| 43 | + - E.g. `/home/user/code/vimperator-labs/vimperator` |
| 44 | +5. Restart Firefox |
| 45 | + |
| 46 | +Hacking |
| 47 | +------- |
| 48 | + |
| 49 | +If you've taken to hacking Vimperator source code, we hope that you'll share |
| 50 | +your changes. In case you do, please keep the following in mind, and we'll be |
| 51 | +happy to accept your patches. |
| 52 | + |
| 53 | +### Documentation |
| 54 | + |
| 55 | +First of all, all new features and all user-visible changes to existing |
| 56 | +features need to be documented. That means editing the appropriate help files |
| 57 | +and adding a NEWS entry where appropriate. When editing the NEWS file, you |
| 58 | +should add your change to the top of the list of changes. If your change alters |
| 59 | +an interface (key binding, command) and is likely to cause trouble, prefix it |
| 60 | +with `IMPORTANT:`, otherwise, place it below the other `IMPORTANT` entries. If |
| 61 | +you're not sure if your change merits a news entry, or if it's important, |
| 62 | +please ask. |
| 63 | + |
| 64 | +### Coding Style |
| 65 | + |
| 66 | +In general: Just look at the existing source code! |
| 67 | + |
| 68 | +We try to target experienced JavaScript developers who do not necessarily need |
| 69 | +to have a good understanding of Vimperator's source code, nor necessarily |
| 70 | +understand in-depth concepts of other languages like Lisp or Python. Therefore, |
| 71 | +the coding style should feel natural to any JavaScript developer. Of course, |
| 72 | +this does not mean, you have to avoid all new JavaScript features like list |
| 73 | +comprehension or generators. Use them, when they make sense, but don't use them |
| 74 | +when the resulting code is hard to read. |
| 75 | + |
| 76 | +#### The most important style issues are: |
| 77 | + |
| 78 | +- Use 4 spaces to indent things, no tabs, not 2, nor 8 spaces. If you use Vim, |
| 79 | + this should be taken care of automatically by the modeline (like the one |
| 80 | + below). |
| 81 | + |
| 82 | +- No trailing whitespace. |
| 83 | + |
| 84 | +- Use `"` for enclosing strings instead of `'`, unless using `'` avoids |
| 85 | + escaping of lots of `"`: |
| 86 | + |
| 87 | + ```javascript |
| 88 | + alert("foo") |
| 89 | + |
| 90 | + alert('foo') |
| 91 | + ``` |
| 92 | + |
| 93 | +- Use `//` regexp literals rather than RegExp constructors, unless you're |
| 94 | + constructing an expression on the fly, or RegExp constructors allow you to |
| 95 | + escape less `/s` than the additional escaping of special characters required |
| 96 | + by string quoting: |
| 97 | + |
| 98 | + ```javascript |
| 99 | + // Good |
| 100 | + /application\/xhtml\+xml/ |
| 101 | + RegExp("http://(www\\.)vimperator.org/(.*)/(.*)") |
| 102 | + |
| 103 | + // Bad |
| 104 | + RegExp("application/xhtml\\+xml") |
| 105 | + /http:\/\/(www\.)vimperator.org\/(.*)\/(.*)/ |
| 106 | + ``` |
| 107 | + |
| 108 | +- Exactly one space after `if/for/while/catch` etc. and after a comma, but none |
| 109 | + after a parenthesis or after a function call: |
| 110 | + |
| 111 | + ```javascript |
| 112 | + for (pre; condition; post) |
| 113 | + alert("foo"); |
| 114 | + ``` |
| 115 | + |
| 116 | +- Bracing is formatted as follows: |
| 117 | + |
| 118 | + ```javascript |
| 119 | + function myFunction () { |
| 120 | + if (foo) |
| 121 | + return bar; |
| 122 | + else { |
| 123 | + baz = false; |
| 124 | + return baz; |
| 125 | + } |
| 126 | + } |
| 127 | + var quux = frob("you", |
| 128 | + { |
| 129 | + a: 1, |
| 130 | + b: 42, |
| 131 | + c: { |
| 132 | + hoopy: "frood" |
| 133 | + } |
| 134 | + }); |
| 135 | + ``` |
| 136 | + |
| 137 | + When in doubt, look for similar code. |
| 138 | + |
| 139 | +- No braces for one-line conditional statements: |
| 140 | + |
| 141 | + ```javascript |
| 142 | + if (foo) |
| 143 | + frob(); |
| 144 | + else |
| 145 | + unfrob(); |
| 146 | + ``` |
| 147 | + |
| 148 | +- Prefer lambda-style functions where suitable: |
| 149 | + |
| 150 | + ```javascript |
| 151 | + // Good |
| 152 | + list.filter(function (elem) elem.good != elem.BAD); |
| 153 | + list.forEach(function (elem) { window.alert(elem); }); |
| 154 | + |
| 155 | + // Bad |
| 156 | + list.filter(function (elem) { return elem.good != elem.BAD }); |
| 157 | + list.forEach(function (elem) window.alert(elem)); |
| 158 | + ``` |
| 159 | + |
| 160 | +- Anonymous function definitions should be formatted with a space after the |
| 161 | + keyword `function`: |
| 162 | + |
| 163 | + ```javascript |
| 164 | + // Good |
| 165 | + function () {} |
| 166 | + |
| 167 | + // Bad |
| 168 | + function() {} |
| 169 | + ``` |
| 170 | +
|
| 171 | +- Prefer the use of `let` over `var` i.e. only use `var` when required. |
| 172 | +
|
| 173 | + For more details, see: |
| 174 | + https://developer.mozilla.org/en/New_in_JavaScript_1.7#Block_scope_with_let |
| 175 | +
|
| 176 | +- Reuse common local variable names. E.g. `elem` is generally used for element, |
| 177 | + `win` for windows, `func` for functions, `ret` for return values etc. |
| 178 | +
|
| 179 | +- Prefer `//` over `/* */` comments (exceptions for big comments are usually |
| 180 | + OK): |
| 181 | + |
| 182 | + ```javascript |
| 183 | + // Good |
| 184 | + if (HACK) // TODO: remove hack |
| 185 | +
|
| 186 | + // Bad |
| 187 | + if (HACK) /* TODO: remove hack */ |
| 188 | + ``` |
| 189 | + |
| 190 | +- Documentation comment blocks use `/** ... */` Wrap these lines at 80 |
| 191 | + characters. |
| 192 | + |
| 193 | +- Only wrap lines if it makes the code obviously clearer. Lines longer than 132 |
| 194 | + characters should probably be broken up rather than wrapped anyway. |
| 195 | + |
| 196 | +- Use UNIX new lines (`\n`), not windows (`\r\n`) or old Mac ones (`\r`). |
| 197 | + |
| 198 | +- Use `Iterator` or `Array#forEach` to iterate over arrays. |
| 199 | + |
| 200 | + `for (let i in ary)` and `for each (let i in ary)` include members in an |
| 201 | + `Array.prototype`, which some extensions alter: |
| 202 | + |
| 203 | + ```javascript |
| 204 | + // Good |
| 205 | + for (let [,elem] in Iterator(ary)) |
| 206 | + for (let [k, v] in Iterator(obj)) |
| 207 | + ary.forEach(function (elem) { ... |
| 208 | +
|
| 209 | + // Bad |
| 210 | + for each (let elem in ary) |
| 211 | + ``` |
| 212 | + |
| 213 | + The exceptions to this rule are for objects with `__iterator__` set, and for |
| 214 | + XML objects (see README.E4X). |
| 215 | + |
| 216 | +- Avoid using `new` with constructors where possible, and use `[]` and |
| 217 | + `{}` rather than `new Array` or `new Object`: |
| 218 | + |
| 219 | + ```javascript |
| 220 | + // Good |
| 221 | + RegExp("^" + foo + "$") |
| 222 | + Function(code) |
| 223 | + new Date |
| 224 | +
|
| 225 | + // Bad |
| 226 | + new RegExp("^" + foo + "$") |
| 227 | + new Function(code) |
| 228 | + Date() // Right if you want a string-representation of the date |
| 229 | + ``` |
| 230 | + |
| 231 | +- Don't use abbreviations for public methods: |
| 232 | +
|
| 233 | + ```javascript |
| 234 | + // Good |
| 235 | + function splitString()... |
| 236 | + let commands = ...; |
| 237 | + let cmds = ...; // Since it's only used locally, abbreviations are ok, but so are the full names |
| 238 | + |
| 239 | + // Bad |
| 240 | + function splitStr() |
| 241 | + ``` |
| 242 | + |
| 243 | +Testing/Optimization |
| 244 | +-------------------- |
| 245 | + |
| 246 | +**TODO:** |
| 247 | + |
| 248 | +- Add some information here about testing/validation/etc. |
| 249 | +- Information about how/when to use `:regressions` might be nice. |
| 250 | +- Additionally, maybe there should be some benchmark information here, |
| 251 | + something to let a developer know what's "too" slow...? Or general guidelines |
| 252 | + about optimization? |
| 253 | + |
| 254 | +<!-- vim: set ft=markdown sw=4 ts=4 sts=4 et ai: --> |
0 commit comments