Bugfix in index mapping; added tests (#35)

cxw42 · Apr 24, 2018 · 1f425a0 · 1f425a0
1 parent 9472e94
commit 1f425a0
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 15 deletions.
diff --git a/tabfern/src/view/model.js b/tabfern/src/view/model.js
@@ -661,20 +661,22 @@
         if(!D.windows.by_node_id(win_node.id, 'isOpen')) return false;
         let nkids = win_node.children.length;
 
-        // #cidx is the number of open tabs we have to skip to get to where
-        // we want to insert.
-
-        let remaining = cidx;   //                     ||
-        let child_idx;          // Note: this was <=,  VV  but I don't recall why.
-        for(child_idx=0; (remaining > 0) && (child_idx < nkids); ++child_idx) {
-                                //                     ^^
-            let child_node_id = win_node.children[child_idx];
-            if(D.tabs.by_node_id(child_node_id, 'isOpen')) {
-                --remaining;
+        // Build a node list as if all the open tabs were packed together
+        let orig_idx = [];
+        win_node.children.forEach( (kid_node_id, kid_idx)=>{
+            if(D.tabs.by_node_id(kid_node_id, 'isOpen')) {
+                orig_idx.push(kid_idx);
+                // TODO break if we've gone far enough?
             }
+        });
+
+        // Pick the cidx from that list
+        if(cidx >= orig_idx.length) {           // New tab off the end
+            return 1 + orig_idx[orig_idx.length-1];
+        } else {                                // Tab that exists
+            return orig_idx[cidx];
         }
 
-        return child_idx;   // jstree index, so 0 <= retval <= nkids
     } //treeIdxByChromeIdx()
 
     /// Convert a tree index to a Chrome tab index in a window,
@@ -683,7 +685,7 @@
     /// @param win_nodey {mixed} The window in question
     /// @param tree_item {mixed} The child node or index whose ctab index we want.
     ///         If string, the node ID; otherwise, the tree index in the tree.
-    /// @pre #tree_idx <= number of children of #win_nodey
+    ///         If numeric, #tree_item <= number of children of #win_nodey
     module.chromeIdxOfTab = function chromeIdxOfTab(win_nodey, tree_item)
     {
         let win_node = T.treeobj.get_node(win_nodey);
@@ -695,7 +697,7 @@
 
         let tree_idx;
         if(typeof tree_item !== 'string') {
-            tree_idx = tree_item;
+            tree_idx = Number(tree_item);
         } else {
             tree_idx = win_node.children.indexOf(tree_item);
             if(tree_idx === -1) return false;

diff --git a/tabfern/test/spec/spec-view-model.js b/tabfern/test/spec/spec-view-model.js
@@ -350,11 +350,29 @@ describe('view/model', function() {
 
         describe('next-to-last tab open',()=>{
             beforeAll(()=>{this.openSomeTabs([this.NTABS-2]);});
+            it('maps open tab->chrome',()=>{
+                expect(M.chromeIdxOfTab(this.win_node_id, this.NTABS-2)).toBe(0);
+            });
+            it('maps open chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 0)).toBe(this.NTABS-2);
+            });
+            it('maps new-tab chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 1)).toBe(this.NTABS-1);
+            });
             afterAll(()=>{this.closeAllTabs();});
         });
 
         describe('last tab open',()=>{
             beforeAll(()=>{this.openSomeTabs([this.NTABS-1]);});
+            it('maps open tab->chrome',()=>{
+                expect(M.chromeIdxOfTab(this.win_node_id, this.NTABS-1)).toBe(0);
+            });
+            it('maps open chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 0)).toBe(this.NTABS-1);
+            });
+            it('maps new-tab chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 1)).toBe(this.NTABS);
+            });
             afterAll(()=>{this.closeAllTabs();});
         });
 
@@ -363,21 +381,75 @@ describe('view/model', function() {
 
         describe('tabs 0 and 1 open',()=>{
             beforeAll(()=>{this.openSomeTabs([0, 1]);});
+
+            it('maps open tab->chrome',()=>{
+                expect(M.chromeIdxOfTab(this.win_node_id, 0)).toBe(0);
+                expect(M.chromeIdxOfTab(this.win_node_id, 1)).toBe(1);
+            });
+            it('maps open chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 0)).toBe(0);
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 1)).toBe(1);
+            });
+            it('maps new-tab chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 2)).toBe(2);
+            });
+
             afterAll(()=>{this.closeAllTabs();});
         });
 
         describe('tabs 1 and 2 open',()=>{
             beforeAll(()=>{this.openSomeTabs([1, 2]);});
+
+            // TODO throughout - failure cases - what about asking for the
+            // Chrome index of a closed tab?
+            it('maps open tab->chrome',()=>{
+                expect(M.chromeIdxOfTab(this.win_node_id, 1)).toBe(0);
+                expect(M.chromeIdxOfTab(this.win_node_id, 2)).toBe(1);
+            });
+            it('maps open chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 0)).toBe(1);
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 1)).toBe(2);
+            });
+            it('maps new-tab chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 2)).toBe(3);
+            });
+
             afterAll(()=>{this.closeAllTabs();});
         });
 
         describe('next-to-last two tabs open',()=>{
-            beforeAll(()=>{this.openSomeTabs([this.NTABS-2, this.NTABS-1]);});
+            beforeAll(()=>{this.openSomeTabs([this.NTABS-3, this.NTABS-2]);});
+
+            it('maps open tab->chrome',()=>{
+                expect(M.chromeIdxOfTab(this.win_node_id, this.NTABS-3)).toBe(0);
+                expect(M.chromeIdxOfTab(this.win_node_id, this.NTABS-2)).toBe(1);
+            });
+            it('maps open chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 0)).toBe(this.NTABS-3);
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 1)).toBe(this.NTABS-2);
+            });
+            it('maps new-tab chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 2)).toBe(this.NTABS-1);
+            });
+
             afterAll(()=>{this.closeAllTabs();});
         });
 
         describe('last two tabs open',()=>{
-            beforeAll(()=>{this.openSomeTabs([this.NTABS-1, this.NTABS]);});
+            beforeAll(()=>{this.openSomeTabs([this.NTABS-2, this.NTABS-1]);});
+
+            it('maps open tab->chrome',()=>{
+                expect(M.chromeIdxOfTab(this.win_node_id, this.NTABS-2)).toBe(0);
+                expect(M.chromeIdxOfTab(this.win_node_id, this.NTABS-1)).toBe(1);
+            });
+            it('maps open chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 0)).toBe(this.NTABS-2);
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 1)).toBe(this.NTABS-1);
+            });
+            it('maps new-tab chrome->tab',()=>{
+                expect(M.treeIdxByChromeIdx(this.win_node_id, 2)).toBe(this.NTABS);
+            });
+
             afterAll(()=>{this.closeAllTabs();});
         });