refactor(cleanup) Remove unused methods from Post, Markerable (#474)

* Remove unused `Post#markersFrom` * Remove unused `Post#cutMarkers` * Remove `Post#_nextMarkerableSection` * Remove unused `Markerable#splitMarker` * Remove FIXME from `Post#_nextLeafSection` * Remove FIXME from dom-utils `addClassName`
bustle · Aug 30, 2016 · 235f7a3 · 235f7a3
1 parent 1cef370
commit 235f7a3
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 190 deletions.
diff --git a/src/js/models/_markerable.js b/src/js/models/_markerable.js
@@ -1,4 +1,4 @@
-import { forEach, filter, reduce } from '../utils/array-utils';
+import { forEach, reduce } from '../utils/array-utils';
 import Set from '../utils/set';
 
 import LinkedList from '../utils/linked-list';
@@ -83,18 +83,6 @@ export default class Markerable extends Section {
     return offset;
   }
 
-  /**
-   * Splits the marker at the offset, filters empty markers from the result,
-   * and replaces this marker with the new non-empty ones
-   * @param {Marker} marker the marker to split
-   * @return {Array} the new markers that replaced `marker`
-   */
-  splitMarker(marker, offset, endOffset=marker.length) {
-    const newMarkers = filter(marker.split(offset, endOffset), m => !m.isBlank);
-    this.markers.splice(marker, 1, newMarkers);
-    return newMarkers;
-  }
-
   // puts clones of this.markers into beforeSection and afterSection,
   // all markers before the marker/offset split go in beforeSection, and all
   // after the marker/offset split go in afterSection

diff --git a/src/js/models/post.js b/src/js/models/post.js
@@ -1,8 +1,9 @@
 import { POST_TYPE } from './types';
 import LinkedList from 'mobiledoc-kit/utils/linked-list';
-import { forEach, compact } from 'mobiledoc-kit/utils/array-utils';
+import { forEach } from 'mobiledoc-kit/utils/array-utils';
 import Set from 'mobiledoc-kit/utils/set';
 import Position from 'mobiledoc-kit/utils/cursor/position';
+import assert from 'mobiledoc-kit/utils/assert';
 
 /**
  * The Post is an in-memory representation of an editor's document.
@@ -94,59 +95,6 @@ class Post {
     return markers;
   }
 
-  cutMarkers(markers) {
-    let firstSection = markers.length && markers[0].section,
-        lastSection  = markers.length && markers[markers.length - 1].section;
-
-    let currentSection = firstSection;
-    let removedSections = [],
-        changedSections = compact([firstSection, lastSection]);
-
-    if (markers.length !== 0) {
-      markers.forEach(marker => {
-        if (marker.section !== currentSection) { // this marker is in a section we haven't seen yet
-          if (marker.section !== firstSection &&
-              marker.section !== lastSection) {
-            // section is wholly contained by markers, and can be removed
-            removedSections.push(marker.section);
-          }
-        }
-
-        currentSection = marker.section;
-        currentSection.markers.remove(marker);
-      });
-
-      if (firstSection !== lastSection) {
-        firstSection.join(lastSection);
-        removedSections.push(lastSection);
-      }
-    }
-
-    return {changedSections, removedSections};
-  }
-
-  /**
-   * Invoke `callbackFn` for all markers between the headMarker and tailMarker (inclusive),
-   * across sections
-   * @private
-   */
-  markersFrom(headMarker, tailMarker, callbackFn) {
-    let currentMarker = headMarker;
-    while (currentMarker) {
-      callbackFn(currentMarker);
-
-      if (currentMarker === tailMarker) {
-        currentMarker = null;
-      } else if (currentMarker.next) {
-        currentMarker = currentMarker.next;
-      } else {
-        let nextSection = this._nextMarkerableSection(currentMarker.section);
-        // FIXME: This will fail across cards
-        currentMarker = nextSection && nextSection.markers.head;
-      }
-    }
-  }
-
   markupsInRange(range) {
     const markups = new Set();
 
@@ -216,30 +164,19 @@ class Post {
     });
   }
 
-  _nextMarkerableSection(section) {
-    let nextSection = this._nextLeafSection(section);
-
-    while (nextSection && !nextSection.isMarkerable) {
-      nextSection = this._nextLeafSection(nextSection);
-    }
-
-    return nextSection;
-  }
-
   // return the next section that has markers after this one,
   // possibly skipping non-markerable sections
   _nextLeafSection(section) {
     if (!section) { return null; }
-    const hasChildren  = s => !!s.items;
-    const firstChild   = s => s.items.head;
 
-    // FIXME this can be refactored to use `isLeafSection`
     const next = section.next;
     if (next) {
-      if (hasChildren(next)) { // e.g. a ListSection
-        return firstChild(next);
-      } else {
+      if (next.isLeafSection) {
         return next;
+      } else if (!!next.items) {
+        return next.items.head;
+      } else {
+        assert('Cannot determine next section from non-leaf-section', false);
       }
     } else if (section.isNested) {
       // if there is no section after this, but this section is a child

diff --git a/src/js/utils/dom-utils.js b/src/js/utils/dom-utils.js
@@ -80,7 +80,6 @@ function getAttributes(element) {
 }
 
 function addClassName(element, className) {
-  // FIXME-IE IE10+
   element.classList.add(className);
 }
 

diff --git a/tests/unit/models/markup-section-test.js b/tests/unit/models/markup-section-test.js
@@ -22,51 +22,6 @@ test('a section can append a marker', (assert) => {
   assert.equal(s1.markers.length, 1);
 });
 
-test('#splitMarker splits the marker at the offset', (assert) => {
-  const m1 = builder.createMarker('hi ');
-  const m2 = builder.createMarker('there!');
-  const s = builder.createMarkupSection('h2', [m1,m2]);
-
-  s.splitMarker(m2, 3);
-  assert.equal(s.markers.length, 3, 'adds a 3rd marker');
-  assert.equal(s.markers.objectAt(0).value, 'hi ', 'original marker unchanged');
-  assert.equal(s.markers.objectAt(1).value, 'the', 'first half of split');
-  assert.equal(s.markers.objectAt(2).value, 're!', 'second half of split');
-});
-
-test('#splitMarker splits the marker at the end offset if provided', (assert) => {
-  const m1 = builder.createMarker('hi ');
-  const m2 = builder.createMarker('there!');
-  const s = builder.createMarkupSection('h2', [m1,m2]);
-
-  s.splitMarker(m2, 1, 3);
-  assert.equal(s.markers.length, 4, 'adds a marker for the split and has one on each side');
-  assert.equal(s.markers.head.value, 'hi ', 'original marker unchanged');
-  assert.equal(s.markers.objectAt(1).value, 't');
-  assert.equal(s.markers.objectAt(2).value, 'he');
-  assert.equal(s.markers.tail.value, 're!');
-});
-
-test('#splitMarker does not create an empty marker if offset=0', (assert) => {
-  const m1 = builder.createMarker('hi ');
-  const m2 = builder.createMarker('there!');
-  const s = builder.createMarkupSection('h2', [m1,m2]);
-
-  s.splitMarker(m2, 0);
-  assert.equal(s.markers.length, 2, 'still 2 markers');
-  assert.equal(s.markers.head.value, 'hi ', 'original 1st marker unchanged');
-  assert.equal(s.markers.tail.value, 'there!', 'original 2nd marker unchanged');
-});
-
-test('#splitMarker does not remove an existing marker when the offset and endOffset are 0', (assert) => {
-  const m1 = builder.createMarker('X');
-  const s = builder.createMarkupSection('p', [m1]);
-  s.splitMarker(m1, 0, 0);
-
-  assert.equal(s.markers.length, 1, 'still 1 marker');
-  assert.equal(s.markers.head.value, 'X', 'still correct marker value');
-});
-
 test('#isBlank returns true if the text length is zero for two markers', (assert) => {
   const m1 = builder.createMarker('');
   const m2 = builder.createMarker('');

diff --git a/tests/unit/models/post-test.js b/tests/unit/models/post-test.js
@@ -6,67 +6,6 @@ const {module, test} = Helpers;
 
 module('Unit: Post');
 
-test('#markersFrom finds markers across markup sections', (assert) => {
-  const post = Helpers.postAbstract.build(({post, markupSection, marker}) =>
-    post([
-      markupSection('p', ['s1m1', 's1m2', 's1m3'].map(t => marker(t))),
-      markupSection('p', ['s2m1', 's2m2', 's2m3'].map(t => marker(t))),
-      markupSection('p', ['s3m1', 's3m2', 's3m3'].map(t => marker(t)))
-    ])
-  );
-
-  let foundMarkers = [];
-
-  const s1m2 = post.sections.objectAt(0).markers.objectAt(1);
-  const s3m2 = post.sections.objectAt(2).markers.objectAt(1);
-
-  assert.equal(s1m2.value, 's1m2', 'precond - find s1m2');
-  assert.equal(s3m2.value, 's3m2', 'precond - find s3m2');
-
-  post.markersFrom(s1m2, s3m2, m => foundMarkers.push(m.value));
-
-  assert.deepEqual(foundMarkers,
-                   [        's1m2', 's1m3',
-                    's2m1', 's2m2', 's2m3',
-                    's3m1', 's3m2'         ],
-                   'iterates correct markers');
-});
-
-test('#markersFrom finds markers across non-homogeneous sections', (assert) => {
-  const post = Helpers.postAbstract.build(builder => {
-    const {post, markupSection, marker, listSection, listItem} = builder;
-
-    return post([
-      markupSection('p', ['s1m1', 's1m2', 's1m3'].map(t => marker(t))),
-      listSection('ul', [
-        listItem(['l1m1', 'l1m2', 'l1m3'].map(t => marker(t))),
-        listItem(['l2m1', 'l2m2', 'l2m3'].map(t => marker(t)))
-      ]),
-      // FIXME test with card section
-      markupSection('p', ['s2m1', 's2m2', 's2m3'].map(t => marker(t))),
-      markupSection('p', ['s3m1', 's3m2', 's3m3'].map(t => marker(t)))
-    ]);
-  });
-
-  let foundMarkers = [];
-
-  const s1m2 = post.sections.objectAt(0).markers.objectAt(1);
-  const s3m2 = post.sections.objectAt(3).markers.objectAt(1);
-
-  assert.equal(s1m2.value, 's1m2', 'precond - find s1m2');
-  assert.equal(s3m2.value, 's3m2', 'precond - find s3m2');
-
-  post.markersFrom(s1m2, s3m2, m => foundMarkers.push(m.value));
-
-  assert.deepEqual(foundMarkers,
-                   [        's1m2', 's1m3',
-                    'l1m1', 'l1m2', 'l1m3',
-                    'l2m1', 'l2m2', 'l2m3',
-                    's2m1', 's2m2', 's2m3',
-                    's3m1', 's3m2'         ],
-                   'iterates correct markers');
-});
-
 test('#walkMarkerableSections finds no section when range contains only a card', (assert) => {
   const post = Helpers.postAbstract.build(builder => {
     const {post, cardSection} = builder;