Issue 7016 - Convert serialization functions into generators

lib/subscriptionClasses.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 107 matching lines @@
     if (value != this._disabled)
     {
       let oldValue = this._disabled;
       this._disabled = value;
       filterNotifier.emit("subscription.disabled", this, value, oldValue);
     }
     return this._disabled;
   },
 
   /**
-   * Serializes the subscription to an array of strings for writing

[Manish Jethani, 2018/10/21 21:51:43]

We could remove the "to an array of strings" part from this comment and keep the
rest of it.

[Jon Sonesen, 2018/10/22 19:27:47]

Acknowledged.

-   * out on the disk.
+   * Serializes the subscription for writing out on disk.
    * @yields {string}
    */
   *serialize()
   {
     let {url, type, _title, _fixedTitle, _disabled} = this;
 
     yield "[Subscription]";
     yield "url=" + url;
 
     if (type)
       yield "type=" + type;
     if (_title)
       yield "title=" + _title;
     if (_fixedTitle)
       yield "fixedTitle=true";
     if (_disabled)
       yield "disabled=true";
   },
 
   *serializeFilters()
   {
-    for (let filter of this.filters)

[Jon Sonesen, 2018/10/22 19:32:40]

I noticed this method goes against our pattern elsewhere. Not sure if it makes
sense here since it can have an empty section when writing to disk, but also
this will ensure uniform behavior for serialization.

[Manish Jethani, 2018/10/22 20:18:01]

Just for the sake of consistency, I wouldn't mind if we added the following line
to the start of the function:

  let {filters} = this;

Even though it does nothing, just for consistency.

[Jon Sonesen, 2018/10/23 02:04:24]

Done.

+    let {filters} = this;
+
+    yield "[Subscription filters]";
+
+    for (let filter of filters)
       yield filter.text.replace(/\[/g, "\\[");
   },
 
   toString()
   {
     return [...this.serialize()].join("\n");
   }
 };
 
 /**
@@ skipping 119 matching lines @@
   /**
    * See Subscription.serialize()
    * @inheritdoc
    */
   *serialize()
   {
     let {defaults, _lastDownload} = this;
 
     yield* Subscription.prototype.serialize.call(this);
 
-    if (defaults && defaults.length)
+    if (defaults)
     {
       yield "defaults=" +
             defaults.filter(
               type => SpecialSubscription.defaultsMap.has(type)
             ).join(" ");
     }
     if (_lastDownload)
       yield "lastDownload=" + _lastDownload;
   }
 });
@@ skipping 277 matching lines @@
     if (errors)
       yield "errors=" + errors;
     if (version)
       yield "version=" + version;
     if (requiredVersion)
       yield "requiredVersion=" + requiredVersion;
     if (downloadCount)
       yield "downloadCount=" + downloadCount;
   }
 });