Noissue - Document the prefs module

lib/prefs.js

Index: lib/prefs.js
===================================================================
--- a/lib/prefs.js
+++ b/lib/prefs.js
@@ -15,35 +15,152 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/** @module prefs */
+
 const keyPrefix = "pref:";
 
+/** @lends module:prefs.Prefs */
 let defaults = Object.create(null);
 let overrides = Object.create(null);
 
+/**
+ * Only for compatibility with core code. Please do not change!
+ *
+ * @type {boolean}
+ */
 defaults.enabled = true;
+/**
+ * The application version as set during intilization. Used to detect updates.

[kzar, 2015/04/16 12:11:14]

Nit: initialization

[kzar, 2015/04/16 13:32:34]

What about these comments?

[Sebastian Noack, 2015/04/16 13:47:06]

I didn't addressed them yet, or do you read "Done"? ;)

[kzar, 2015/04/16 13:48:55]

You quite commonly say "What about this comment" on my reviews should I not
reply to one. I assumed you would expect the same.

[Sebastian Noack, 2015/04/16 13:51:43]

Done.

[Sebastian Noack, 2015/04/16 13:51:43]

Only if you uploaded a new patch set, and I have reason to assume that you
overlooked those comments or try to cheat around addressing them. ;P

Done.

[kzar, 2015/04/16 13:54:50]

You had uploaded a new patch set though...

[Sebastian Noack, 2015/04/16 13:57:02]

... before you published your comments. ;)

+ *
+ * @type {string}
+ */
 defaults.currentVersion = "";
+/**
+ * Only for compatibility with core code. Please do not change!

[kzar, 2015/04/16 12:11:14]

It would be nice to add a little detail for all these ones that we ask not to be
changed. It might help me to grok the core code later for example.

[Sebastian Noack, 2015/04/16 12:52:00]

Those are already documented under https://adblockplus.org/en/preferences.
Duplicating the documentation or even linking to it here, would be misleading,
since those preferences don't have the same effect here, but changing them would
merely break the extension.

[kzar, 2015/04/16 13:32:34]

Fair enough but in that case how about adding a "@see
https://adblockplus.org/en/preferences" to them?

[Sebastian Noack, 2015/04/16 13:47:06]

I do for preferences that have an effect here. But as I said, those preferences
don't have the documented effect on Chrome, Opera and Safari, but would merely
break the extension. So there is no point in linking to the Firefox
documentation here. All that somebody working on this code needs to know is that
he must not touch those preferences.

+ *
+ * @type {string}
+ */
 defaults.data_directory = "";
+/**
+ * @see https://adblockplus.org/en/preferences#patternsbackups
+ * @type {number}
+ */
 defaults.patternsbackups = 5;
+/**
+ * @see https://adblockplus.org/en/preferences#patternsbackupinterval
+ * @type {number}
+ */
 defaults.patternsbackupinterval = 24;
+/**
+ * Only for compatibility with core code. Please do not change!
+ *
+ * @type {boolean}
+ */
 defaults.savestats = false;
+/**
+ * Only for compatibility with core code. Please do not change!
+ *
+ * @type {boolean}
+ */
 defaults.privateBrowsing = false;
+/**
+ * @see https://adblockplus.org/en/preferences#subscriptions_fallbackerrors
+ * @type {number}
+ */
 defaults.subscriptions_fallbackerrors = 5;
+/**
+ * @see https://adblockplus.org/en/preferences#subscriptions_fallbackurl
+ * @type {string}
+ */
 defaults.subscriptions_fallbackurl = "https://adblockplus.org/getSubscription?version=%VERSION%&url=%SUBSCRIPTION%&downloadURL=%URL%&error=%ERROR%&channelStatus=%CHANNELSTATUS%&responseStatus=%RESPONSESTATUS%";
+/**
+ * @see https://adblockplus.org/en/preferences#subscriptions_autoupdate
+ * @type {boolean}
+ */
 defaults.subscriptions_autoupdate = true;
+/**
+ * @see https://adblockplus.org/en/preferences#subscriptions_exceptionsurl
+ * @type {string}
+ */
 defaults.subscriptions_exceptionsurl = "https://easylist-downloads.adblockplus.org/exceptionrules.txt";
+/**
+ * @see https://adblockplus.org/en/preferences#subscriptions_antiadblockurl
+ * @type {string}
+ */
 defaults.subscriptions_antiadblockurl = "https://easylist-downloads.adblockplus.org/antiadblockfilters.txt";
+/**
+ * @see https://adblockplus.org/en/preferences#documentation_link
+ * @type {string}
+ */
 defaults.documentation_link = "https://adblockplus.org/redirect?link=%LINK%&lang=%LANG%";
+/**
+ * @see https://adblockplus.org/en/preferences#notificationdata
+ * @type {object}
+ */
 defaults.notificationdata = {};
+/**
+ * @see https://adblockplus.org/en/preferences#notificationurl
+ * @type {string}
+ */
 defaults.notificationurl = "https://notification.adblockplus.org/notification.json";
+/**
+ * The total number of ads blocked on all pages.
+ *
+ * @type {object}
+ * @property {number} [blocked]
+ */
 defaults.stats_total = {};
+/**
+ * Whether to show a badge in the toolbar icon indicating the number of blocked ads.
+ *
+ * @type {boolean}
+ */
 defaults.show_statsinicon = true;
+/**
+ * Whether to show the number of blocked ads in the popup.
+ *
+ * @type {boolean}
+ */
 defaults.show_statsinpopup = true;
+/**
+ * Whether to show the "Block element" context menu entry.
+ *
+ * @type {boolean}
+ */
 defaults.shouldShowBlockElementMenu = true;
+/**
+ * Whether to collapse placeholders for blocked elements.
+ *
+ * @type {boolean}
+ */
 defaults.hidePlaceholders = true;
+/**
+ * Whether to show the first run page. This preference isn't
+ * set by the extension but can be pre-configured externally.
+ *
+ * @type {boolean}
+ */
 defaults.suppress_first_run_page = false;
 
+/**
+  * @namespace
+  * @static
+  */
 let Prefs = exports.Prefs = {
+  /**
+   * Fired when the value of a preference changes.
+   *
+   * @event
+   * @property {string} pref The name of the preference that changed.
+   */
   onChanged: new ext._EventTarget(),
+
+  /**
+   * Fired when all preferences have been loaded. You must wait for
+   * this event before using preferences during extension intilization.

[kzar, 2015/04/16 12:11:14]

Nit: initialization

[Sebastian Noack, 2015/04/16 13:51:43]

Done.

+   *
+   * @event
+   */
   onLoaded: new ext._EventTarget()
 };