From 1e942be639aebefb96e8fc6803fe7e1d80da8190 Mon Sep 17 00:00:00 2001 From: "Jasper St. Pierre" Date: Fri, 7 Oct 2011 19:46:19 -0400 Subject: [PATCH] Add a new code style/hacking guide Written in Markdown style. Somewhat based off of the gjs code style guide found at http://git.gnome.org/browse/gjs/plain/doc/Style_Guide.txt. https://bugzilla.gnome.org/show_bug.cgi?id=661241 --- HACKING | 331 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 331 insertions(+) create mode 100644 HACKING diff --git a/HACKING b/HACKING new file mode 100644 index 000000000..fbdcdb61e --- /dev/null +++ b/HACKING @@ -0,0 +1,331 @@ +Coding guide +============ + +Our goal is to have all JavaScript code in GNOME follow a consistent style. In +a dynamic language like JavaScript, it is essential to be rigorous about style +(and unit tests), or you rapidly end up with a spaghetti-code mess. + +A quick note +------------ + +Life isn't fun if you can't break the rules. If a rule seems unnecessarily +restrictive while you're coding, ignore it, and let the patch reviewer decide +what to do. + +Indentation and whitespace +-------------------------- + +Use four-space indents. Braces are on the same line as their associated +statements. You should only omit braces if *both* sides of the statement are +on one line. + +* One space after the `function` keyword. No space between the function name +* in a declaration or a call. One space before the parens in the `if` +* statements, or `while`, or `for` loops. + + function foo(a, b) { + let bar; + + if (a > b) + bar = do_thing(a); + else + bar = do_thing(b); + + if (var == 5) { + for (let i = 0; i < 10; i++) { + print(i); + } + } else { + print(20); + } + } + +Semicolons +---------- + +JavaScript allows omitting semicolons at the end of lines, but don't. Always +end statements with a semicolon. + +js2-mode +-------- + +If using Emacs, do not use js2-mode. It is outdated and hasn't worked for a +while. emacs now has a built-in JavaScript mode, js-mode, based on +espresso-mode. It is the de facto emacs mode for JavaScript. + +File naming and creation +------------------------ + +For JavaScript files, use lowerCamelCase-style names, with a `.js` extension. + +We only use C where gjs/gobject-introspection is not available for the task, or +where C would be cleaner. To work around limitations in +gjs/gobject-introspection itself, add a new method in `shell-util.[ch]`. + +Like many other GNOME projects, we prefix our C source filenames with the +library name followed by a dash, e.g. `shell-app-system.c`. Create a +`-private.h` header when you want to share code internally in the +library. These headers are not installed, distributed or introspected. + +Imports +------- + +Use UpperCamelCase when importing modules to distinguish them from ordinary +variables, e.g. + + const GLib = imports.gi.GLib; + +Imports should be categorized into one of two places. The top-most import block +should contain only "environment imports". These are either modules from +gobject-introspection or modules added by gjs itself. + +The second block of imports should contain only "application imports". These +are the JS code that is in the gnome-shell codebase, +e.g. `imports.ui.popupMenu`. + +Each import block should be sorted alphabetically. Don't import modules you +don't use. + + const GLib = imports.gi.GLib; + const Gio = imports.gi.Gio; + const Lang = imports.lang; + const St = imports.gi.St; + + const Main = imports.ui.main; + const Params = imports.misc.params; + const Tweener = imports.ui.tweener; + const Util = imports.misc.util; + +The alphabetical ordering should be done independently of the location of the +location. Never reference `imports` in actual code. + +Constants +--------- + +We use CONSTANTS_CASE to define constants. All constants should be directly +under the imports: + + const MY_DBUS_INTERFACE = 'org.my.Interface'; + +Variable declaration +-------------------- + +Always use either `const` or `let` when defining a variable. + + // Iterating over an array + for (let i = 0; i < arr.length; ++i) { + let item = arr[i]; + } + + // Iterating over an object's properties + for (let prop in someobj) { + ... + } + +If you use "var" then the variable is added to function scope, not block scope. +See [What's new in JavaScript 1.7](https://developer.mozilla.org/en/JavaScript/New_in_JavaScript/1.7#Block_scope_with_let_%28Merge_into_let_Statement%29) + +Classes +------- + +There are many approaches to classes in JavaScript. We use our own class framework +(sigh), which is built in gjs. The advantage is that it supports inheriting from +GObjects, although this feature isn't used very often in the Shell itself. + + const IconLabelMenuItem = new Lang.Class({ + Name: 'IconLabelMenuItem', + Extends: PopupMenu.PopupMenuBaseItem, + + _init: function(icon, label) { + this.parent({ reactive: false }); + this.addActor(icon); + this.addActor(label); + }, + + open: function() { + log("menu opened!"); + } + }); + +* 'Name' is required. 'Extends' is optional. If you leave it out, you will + automatically inherit from Object. + +* Leave a blank line between the "class header" (Name, Extends, and other + things) and the "class body" (methods). Leave a blank line between each + method. + +* No space before the colon, one space after. + +* No trailing comma after the last item. + +* Make sure to use a semicolon after the closing paren to the class. It's + still a giant function call, even though it may resemble a more + conventional syntax. + +GObject Introspection +--------------------- + +GObject Introspection is a powerful feature that allows us to have native +bindings for almost any library built around GObject. If a library requires +you to inherit from a type to use it, you can do so: + + const MyClutterActor = new Lang.Class({ + Name: 'MyClutterActor', + Extends: Clutter.Actor, + + vfunc_get_preferred_width: function(actor, forHeight) { + return [100, 100]; + }, + + vfunc_get_preferred_height: function(actor, forWidth) { + return [100, 100]; + }, + + vfunc_paint: function(actor) { + let alloc = this.get_allocation_box(); + Cogl.set_source_color4ub(255, 0, 0, 255); + Cogl.rectangle(alloc.x1, alloc.y1, + alloc.x2, alloc.y2); + } + }); + +Translatable strings, `environment.js` +-------------------------------------- + +We use gettext to translate the GNOME Shell into all the languages that GNOME +supports. The `gettext` function is aliased globally as `_`, you do not need to +explicitly import it. This is done through some magic in the +[environment.js](http://git.gnome.org/browse/gnome-shell/tree/js/ui/environment.js) +file. If you can't find a method that's used, it's probably either in gjs itself +or installed on the global object from the Environment. + +Use 'single quotes' for programming strings that should not be translated +and "double quotes" for strings that the user may see. This allows us to +quickly find untranslated or mistranslated strings by grepping through the +sources for double quotes without a gettext call around them. + +`actor` and `_delegate` +----------------------- + +gjs allows us to set so-called "expando properties" on introspected objects, +allowing us to treat them like any other. Because the Shell was built before +you could inherit from GTypes natively in JS, we usually have a wrapper class +that has a property called `actor`. We call this wrapper class the "delegate". + +We sometimes use expando properties to set a property called `_delegate` on +the actor itself: + + const MyClass = new Lang.Class({ + Name: 'MyClass', + + _init: function() { + this.actor = new St.Button({ text: "This is a button" }); + this.actor._delegate = this; + + this.actor.connect('clicked', Lang.bind(this, this._onClicked)); + }, + + _onClicked: function(actor) { + actor.set_label("You clicked the button!"); + } + }); + +The 'delegate' property is important for anything which trying to get the +delegate object from an associated actor. For instance, the drag and drop +system calls the `handleDragOver` function on the delegate of a "drop target" +when the user drags an item over it. If you do not set the `_delegate` +property, your actor will not be able to be dropped onto. + +Functional style +---------------- + +JavaScript Array objects offer a lot of common functional programming +capabilities such as forEach, map, filter and so on. You can use these when +they make sense, but please don't have a spaghetti mess of function programming +messed in a procedural style. Use your best judgment. + +Closures +-------- + +`this` will not be captured in a closure, it is relative to how the closure is +invoked, not to the value of this where the closure is created, because "this" +is a keyword with a value passed in at function invocation time, it is not a +variable that can be captured in closures. + +All closures should be wrapped with a Lang.bind. + + const Lang = imports.lang; + + let closure = Lang.bind(this, function() { this._fnorbate(); }); + +A more realistic example would be connecting to a signal on a method of a +prototype: + + const Lang = imports.lang; + const FnorbLib = imports.fborbLib; + + const MyClass = new Lang.Class({ + _init: function() { + let fnorb = new FnorbLib.Fnorb(); + fnorb.connect('frobate', Lang.bind(this, this._onFnorbFrobate)); + }, + + _onFnorbFrobate: function(fnorb) { + this._updateFnorb(); + } + }); + +Object literal syntax +--------------------- + +In JavaScript, these are equivalent: + + foo = { 'bar': 42 }; + foo = { bar: 42 }; + +and so are these: + + var b = foo['bar']; + var b = foo.bar; + +If your usage of an object is like an object, then you're defining "member +variables." For member variables, use the no-quotes no-brackets syntax: `{ bar: +42 }` `foo.bar`. + +If your usage of an object is like a hash table (and thus conceptually the keys +can have special chars in them), don't use quotes, but use brackets: `{ bar: 42 +}`, `foo['bar']`. + +Getters, setters, and Tweener +----------------------------- + +Getters and setters should be used when you are dealing with an API that is +designed around setting properties, like Tweener. If you want to animate an +arbitrary property, create a getter and setter, and use Tweener to animate the +property. + + const ANIMATION_TIME = 2000; + + const MyClass = new Lang.Class({ + Name: 'MyClass', + + _init: function() { + this.actor = new St.BoxLayout(); + this._position = 0; + }, + + get position() { + return this._position; + }, + + set position(value) { + this._position = value; + this.actor.set_position(value, value); + } + }); + + let myThing = new MyClass(); + Tweener.addTween(myThing, + { position: 100, + time: ANIMATION_TIME, + transition: 'easeOutQuad' });