Filterline: Some cleanup (honestbleeps#4793)

* Filterline: Some cleanup & consistent case for class variables

* Add stricter typing

lib/modules/filteReddit.js

@@ -357,9 +357,17 @@ You can use the command line to manipulate Filterline. Enter it by pressing the
  },
 };
 
+export type FilterStorageValues = {|
+ type: string,
+ state: boolean | null,
+ criterion?: string,
+ conditions?: {},
+ undeletable?: boolean,
+|};
+
 type LineState = {
  visible?: boolean,
- filters?: {},
+ filters?: { [id: string]: FilterStorageValues },
 };
 
 const pageID = fullLocation();

lib/modules/filteReddit/Case.js

@@ -34,14 +34,14 @@ export class Case {
  return { conditions, state };
  }
 
- static criterionToConditions(criterion: ?string): ?* {
- if (!criterion && this.pattern && !this.pattern.startsWith('[')) {
- throw new Error('Requires criterion');
+ static criterionToConditions(criterion: string): * {
+ const parse = this.parseCriterion && this.parseCriterion.bind(this);
+ if (!parse) {
+ throw new Error('Does not accept criterion');
  }
 
- const parse = this.parseCriterion && this.parseCriterion.bind(this);
- if (!criterion || !parse) {
- return;
+ if (!criterion && this.pattern && !this.pattern.startsWith('[')) {
+ throw new Error('Requires criterion');
  }
 
  const parts = criterion.split(' & ');
@@ -120,19 +120,19 @@ export class Case {
  value: *;
  +evaluate: (thing: ?Thing, values: ?*[]) => boolean | Promise<boolean>;
 
- observers: ?Set<{ refreshThing: (thing: Thing) => void }> = this.onObserve ? new Set() : null;
- +onObserve: ?() => ?boolean; // `true` → `refreshThing` callback registered
+ observers: ?Set<{ refresh: (thing?: Thing) => void }> = this.onObserve ? new Set() : null;
+ +onObserve: ?() => ?boolean; // `true` → `refresh` callback registered
  observe(observer: *): boolean { // `true` → observer added
  if (!this.observers || this.observers.has(observer)) return false;
  const status = this.onObserve && this.onObserve() || false;
  if (status) this.observers.add(observer);
  return status;
  }
 
- refreshThing(thing: Thing) {
+ refresh(thing?: Thing) {
  if (!this.observers) return;
  for (const o of this.observers) {
- o.refreshThing(thing);
+ o.refresh(thing);
  }
  }
 }

lib/modules/filteReddit/ExternalFilter.js

@@ -9,9 +9,9 @@ import { Filter } from './Filter';
 export class ExternalFilter extends Filter {
  createElement() {
  this.element = string.html`
- <div class="res-filterline-external-filter" type="${this.baseCase.type}">
- ${string.safe(SettingsNavigation.makeUrlHashLink(FilteReddit.module.moduleID, this.baseCase.type, ' ', 'gearIcon'))}
- <div>${this.baseCase.type}</div>
+ <div class="res-filterline-external-filter" type="${this.BaseCase.type}">
+ ${string.safe(SettingsNavigation.makeUrlHashLink(FilteReddit.module.moduleID, this.BaseCase.type, ' ', 'gearIcon'))}
+ <div>${this.BaseCase.type}</div>
  </div>
  `;
  this.element.appendChild(
@@ -20,16 +20,16 @@ export class ExternalFilter extends Filter {
  }
 
  async showThingFilterReason(thing: *) {
- thing.element.setAttribute('filter-reason', `specified by filter ${this.baseCase.type}`);
+ thing.element.setAttribute('filter-reason', `specified by filter ${this.BaseCase.type}`);
 
  if (!Array.isArray((this.case.conditions: any).of)) throw new Error('Must be a group case');
- const entry = await asyncFind((this.case.conditions: any).of, v => this.baseCase.fromConditions(v).evaluate(thing));
+ const entry = await asyncFind((this.case.conditions: any).of, v => this.BaseCase.fromConditions(v).evaluate(thing));
 
  $('<span>', {
  class: 'res-filter-remove-entry',
  title: JSON.stringify(entry, null, ' '),
  click: () => {
- FilteReddit.removeExternalFilterEntry(this.baseCase.type, entry);
+ FilteReddit.removeExternalFilterEntry(this.BaseCase.type, entry);
  this.update(this.state, null, false);
  },
  }).prependTo(thing.element);

lib/modules/filteReddit/Filter.js

@@ -5,6 +5,7 @@ import type { Thing } from '../../utils';
 import {
  fastAsync,
 } from '../../utils';
+import type { FilterStorageValues } from '../filteReddit';
 import { Case } from './Case';
 import type { Filterline } from './Filterline';
 
@@ -15,17 +16,17 @@ export class Filter {
 
  updatePromise: ?Promise<*>;
 
- baseCase: Class<Case>;
+ BaseCase: Class<Case>;
  case: Case;
  state: boolean | null;
 
  element: HTMLElement;
 
- constructor(id: *, baseCase: *, conditions: *, state: *, undeletable: * = false) {
+ constructor(id: *, BaseCase: *, conditions: * = null, state: * = null, undeletable: * = false) {
  this.id = id;
- this.baseCase = baseCase;
+ this.BaseCase = BaseCase;
  this.state = state;
- this.setCase(baseCase.fromConditions(conditions));
+ this.setCase(BaseCase.fromConditions(conditions));
  this.undeletable = undeletable;
  }
 
@@ -37,17 +38,18 @@ export class Filter {
 
  getStateText(state: ?boolean = this.state, cased: Case = this.case) {
  return state === false ?
- cased.falseText || `¬ ${this.baseCase.text.toLowerCase()}` :
- cased.trueText || this.baseCase.text.toLowerCase();
+ cased.falseText || `¬ ${this.BaseCase.text.toLowerCase()}` :
+ cased.trueText || this.BaseCase.text.toLowerCase();
  }
 
  getSaveValues() {
- if (this.baseCase.variant === 'basic') {
- return { type: this.baseCase.type, state: this.state, conditions: this.case.conditions };
- } else {
- // The conditions are found in the type's defaultConditions
- return { type: this.baseCase.type, state: this.state };
+ const values: FilterStorageValues = { type: this.BaseCase.type, state: this.state };
+
+ if (this.BaseCase.variant === 'basic') {
+ values.conditions = this.case.conditions;
  }
+
+ return values;
  }
 
  clear() {
@@ -65,7 +67,7 @@ export class Filter {
  }
 
  update(state: *, conditions?: *, describeOnly: boolean = false) {
- const cased = typeof conditions === 'undefined' ? this.case : this.baseCase.fromConditions(conditions);
+ const cased = typeof conditions === 'undefined' ? this.case : this.BaseCase.fromConditions(conditions);
 
  if (describeOnly) {
  cased.validate();
@@ -75,13 +77,17 @@ export class Filter {
  if (state !== this.state || this.case !== cased) {
  this.state = state;
  this.setCase(cased);
- if (this.parent) this.updatePromise = Promise.all(this.parent.refreshAll(this));
- if (this.parent) this.parent.save();
+ this.refresh();
  }
  }
 
- refreshThing = (thing: Thing) => {
- if (this.parent) this.parent.refreshThing(thing, this);
+ refresh = (thing?: Thing) => {
+ if (this.parent) {
+ this.updatePromise = thing ?
+ this.parent.refreshThing(thing, this) :
+ Promise.all(this.parent.refreshAll(this));
+ this.parent.save();
+ }
  };
 
  async updateByInputConstruction({
@@ -103,9 +109,9 @@ export class Filter {
  let state, conditions;
 
  if (fromSelected) {
- ({ state, conditions } = await this.baseCase.getSelectedEntryValue());
+ ({ state, conditions } = await this.BaseCase.getSelectedEntryValue());
  } else {
- conditions = this.baseCase.criterionToConditions(criterion);
+ conditions = criterion ? this.BaseCase.criterionToConditions(criterion) : null;
  state = this.state;
  }
 
@@ -126,7 +132,7 @@ export class Filter {
  $('<span>', {
  class: 'res-filter-remove-entry',
  title: JSON.stringify(this.case.conditions, null, ' '),
- click: () => { this.update(null); this.refreshThing(thing); },
+ click: () => { this.update(null); this.refresh(thing); },
  }).prependTo(thing.element);
  }
 

lib/modules/filteReddit/Filterline.js

@@ -30,13 +30,10 @@ import type { Filter } from './Filter';
 import type { Case } from './Case';
 
 type NewFilterOpts = {|
- add?: boolean,
- criterion?: string,
- conditions?: *,
  id?: string,
+ add?: boolean,
  save?: boolean,
- state?: *,
- undeletable?: boolean,
+ ...FilteReddit.FilterStorageValues,
 |};
 
 export class Filterline {
@@ -98,8 +95,8 @@ export class Filterline {
  }
  }
 
- getFiltersOfCase(caseClass: *) {
- return this.filters.filter(v => v.baseCase === caseClass);
+ getFiltersOfCase(CaseClass: *) {
+ return this.filters.filter(v => v.BaseCase === CaseClass);
  }
 
  getPickable(): Class<Case>[] {
@@ -146,11 +143,11 @@ export class Filterline {
 
  // `Cases.Group` is separated
  const dp = _.groupBy(_.without(this.getPickable(), Cases.Group), v => v.variant);
- for (const [name, caseClasses] of Object.entries(dp)) {
+ for (const [name, CaseClasses] of Object.entries(dp)) {
  addDetails(`New ${name} filter`, `res-filterline-new-${name}`,
- ...caseClasses
+ ...CaseClasses
  .sort((a, b) => a.type.localeCompare(b.type))
- .map(caseClass => this.createNewFilterElement(caseClass))
+ .map(CaseClass => this.createNewFilterElement(CaseClass))
  );
  }
 
@@ -197,26 +194,25 @@ export class Filterline {
  });
  }
 
- createNewFilterElement(caseClass: Class<Case>, text: string = caseClass.text, newOpts?: NewFilterOpts) {
+ createNewFilterElement(CaseClass: Class<Case>, text: string = CaseClass.text, newOpts?: $Shape<NewFilterOpts>) {
  let fromSelected = false;
 
- const element = string.html`<div class="res-filterline-dropdown-action res-filterline-filter-new" type="${caseClass.type}">${text}</div>`;
+ const element = string.html`<div class="res-filterline-dropdown-action res-filterline-filter-new" type="${CaseClass.type}">${text}</div>`;
  element.addEventListener('click', () => {
- const existing = caseClass.unique && this.getFiltersOfCase(caseClass)[0];
+ const existing = CaseClass.unique && this.getFiltersOfCase(CaseClass)[0];
  let filter;
  if (existing) {
  if (!(existing instanceof LineFilter)) throw new Error();
  filter = existing;
  } else {
- // $FlowIssue
- filter = downcast(this.createFilterFromType(caseClass.type, { add: true, ...newOpts }), LineFilter);
+ filter = downcast(this.createFilter({ type: CaseClass.type, add: true, ...newOpts }), LineFilter);
  }
 
  filter.updateByInputConstruction({ fromSelected });
  filter.showInfocard(true);
  });
 
- if (caseClass.thingToCriterion || !caseClass.defaultConditions) {
+ if (CaseClass.thingToCriterion || !CaseClass.defaultConditions) {
  const c = string.html`<div class="res-filterline-filter-new-from-selected" title="From selected entry"></div>`;
  c.addEventListener('click', () => {
  fromSelected = true;
@@ -273,11 +269,11 @@ export class Filterline {
  }
 
  restoreState(filters: *) {
- for (const [id, { type, state, criterion, conditions, undeletable }] of Object.entries(filters)) {
+ for (const [id, opts] of Object.entries(filters)) {
  try {
- this.createFilterFromType(type, { add: true, id, state, criterion, conditions, undeletable, save: false });
+ this.createFilter({ ...opts, id, add: true, save: false });
  } catch (e) {
- console.error('Could not create filter', type, 'deleting', e);
+ console.error('Could not create filter', opts, 'deleting', e);
  this.storage.deletePath('filters', id);
  }
  }
@@ -325,7 +321,7 @@ export class Filterline {
  const lastFilter = _.last(this.getFiltersOfCase(bestMatch));
  filter = lastFilter && !asNewFilter ?
  lastFilter :
- this.createFilterFromType(bestMatch.type);
+ this.createFilter({ type: bestMatch.type });
 
  const actionDescription = await filter.updateByInputConstruction(deconstructed, true);
  /*:: if (!actionDescription ) throw new Error(); */
@@ -370,25 +366,24 @@ export class Filterline {
  return { getTip: getTip.bind(this), executeCommand: executeCommand.bind(this) };
  }
 
- createFilterFromType(
- type: *,
- {
- add,
- criterion,
- conditions,
- id = `~${performance.timing.navigationStart + performance.now()}`, // timestamp, so that filters will restored in the same order as they initially were created
- save = true,
- state = null,
- undeletable,
- }: NewFilterOpts = {}) {
- const caseClass = Cases.get(type);
- if (caseClass.unique && this.getFiltersOfCase(caseClass).length) throw new Error('Cannot create new instances of unique filters');
-
- if (!conditions && typeof criterion !== 'undefined') {
- conditions = caseClass.criterionToConditions(criterion);
+ createFilter({
+ id = `~${performance.timing.navigationStart + performance.now()}`, // timestamp, so that filters will restored in the same order as they initially were created
+ add = false,
+ save = true,
+ type,
+ criterion,
+ conditions,
+ state,
+ undeletable,
+ }: $Shape<NewFilterOpts>) {
+ const CaseClass = Cases.get(type);
+ if (CaseClass.unique && this.getFiltersOfCase(CaseClass).length) throw new Error('Cannot create new instances of unique filters');
+
+ if (!conditions && criterion) {
+ conditions = CaseClass.criterionToConditions(criterion);
  }
 
- const filter = new (caseClass.variant === 'external' ? ExternalFilter : LineFilter)(id, caseClass, conditions, state, undeletable);
+ const filter = new (CaseClass.variant === 'external' ? ExternalFilter : LineFilter)(id, CaseClass, conditions, state, undeletable);
 
  if (add) {
  this.addFilter(filter);
@@ -399,7 +394,7 @@ export class Filterline {
  }
 
  addFilter(filter: Filter) {
- if (filter.baseCase === Cases.Inert || filter.baseCase.disabled) {
+ if (filter.BaseCase === Cases.Inert || filter.BaseCase.disabled) {
  this.inertFilters.push(filter);
  return;
  }