From eb929cb7a205398c2aa6df37ca07ae66f876853e Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@github.com>
Date: Tue, 30 Sep 2014 11:44:48 -0600
Subject: [PATCH] Honor item specificity while still preserving addition order
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Rather than using order to specify item precedence, we now construct
a set of menu items for each element traversing upward from the target.
When merging items for a given element, we pass the specificity to the
merge function, which uses it to decide whether or not to clobber
existing items. When assembling the overall menu, we don’t ever clobber
to ensure that items added for elements closer to the target always win
over items matching further up the tree.
---
 spec/context-menu-manager-spec.coffee |  5 +++++
 src/context-menu-manager.coffee       | 23 +++++++++--------------
 src/menu-helpers.coffee               | 12 ++++++++----
 3 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/spec/context-menu-manager-spec.coffee b/spec/context-menu-manager-spec.coffee
index a0cb69485..9c3c02175 100644
--- a/spec/context-menu-manager-spec.coffee
+++ b/spec/context-menu-manager-spec.coffee
@@ -68,6 +68,8 @@ describe "ContextMenuManager", ->
         '.grandchild.foo': [{label: 'A', command: 'b'}]
       disposable3 = contextMenu.add
         '.grandchild': [{label: 'A', command: 'c'}]
+      disposable4 = contextMenu.add
+        '.child': [{label: 'A', command: 'd'}]
 
       expect(contextMenu.templateForElement(grandchild)).toEqual [{label: 'A', command: 'b'}]
 
@@ -77,6 +79,9 @@ describe "ContextMenuManager", ->
       disposable3.dispose()
       expect(contextMenu.templateForElement(grandchild)).toEqual [{label: 'A', command: 'a'}]
 
+      disposable1.dispose()
+      expect(contextMenu.templateForElement(grandchild)).toEqual [{label: 'A', command: 'd'}]
+
     it "allows multiple separators", ->
       contextMenu.add
         '.grandchild': [
diff --git a/src/context-menu-manager.coffee b/src/context-menu-manager.coffee
index f39cd8b18..1f00c5e7a 100644
--- a/src/context-menu-manager.coffee
+++ b/src/context-menu-manager.coffee
@@ -10,7 +10,6 @@ Grim = require 'grim'
 MenuHelpers = require './menu-helpers'
 
 SpecificityCache = {}
-SequenceCount = 0
 
 # Extended: Provides a registry for commands that you'd like to appear in the
 # context menu.
@@ -91,7 +90,7 @@ class ContextMenuManager
     addedItemSets = []
 
     for selector, items of itemsBySelector
-      itemSet = new ContextMenuItemSet(selector, items.slice())
+      itemSet = new ContextMenuItemSet(selector, items)
       addedItemSets.push(itemSet)
       @itemSets.push(itemSet)
 
@@ -107,19 +106,21 @@ class ContextMenuManager
     currentTarget = event.target
 
     while currentTarget?
+      currentTargetItems = []
       matchingItemSets =
-        @itemSets
-          .filter (itemSet) -> currentTarget.webkitMatchesSelector(itemSet.selector)
-          .sort (a, b) -> a.compare(b)
+        @itemSets.filter (itemSet) -> currentTarget.webkitMatchesSelector(itemSet.selector)
 
-      for {items} in matchingItemSets
-        for item in items
+      for itemSet in matchingItemSets
+        for item in itemSet.items
           continue if item.devMode and not @devMode
           item = Object.create(item)
           if typeof item.shouldDisplay is 'function'
             continue unless item.shouldDisplay(event)
           item.created?(event)
-          MenuHelpers.merge(template, MenuHelpers.cloneMenuItem(item))
+          MenuHelpers.merge(currentTargetItems, MenuHelpers.cloneMenuItem(item), itemSet.specificity)
+
+      for item in currentTargetItems
+        MenuHelpers.merge(template, item, false)
 
       currentTarget = currentTarget.parentElement
 
@@ -167,9 +168,3 @@ class ContextMenuManager
 class ContextMenuItemSet
   constructor: (@selector, @items) ->
     @specificity = (SpecificityCache[@selector] ?= specificity(@selector))
-    @sequenceNumber = SequenceCount++
-
-  # more specific / recent item sets sort later, because we clobber existing menu items
-  compare: (other) ->
-    @specificity - other.specificity or
-      @sequenceNumber - other.sequenceNumber
diff --git a/src/menu-helpers.coffee b/src/menu-helpers.coffee
index c7b449112..a8fd26a50 100644
--- a/src/menu-helpers.coffee
+++ b/src/menu-helpers.coffee
@@ -1,14 +1,18 @@
 _ = require 'underscore-plus'
 
-merge = (menu, item) ->
+ItemSpecificities = new WeakMap
+
+merge = (menu, item, itemSpecificity=Infinity) ->
+  ItemSpecificities.set(item, itemSpecificity) if itemSpecificity
   matchingItemIndex = findMatchingItemIndex(menu, item)
   matchingItem = menu[matchingItemIndex] unless matchingItemIndex is - 1
 
   if matchingItem?
     if item.submenu?
-      merge(matchingItem.submenu, submenuItem) for submenuItem in item.submenu
-    else
-      menu[matchingItemIndex] = item
+      merge(matchingItem.submenu, submenuItem, itemSpecificity) for submenuItem in item.submenu
+    else if itemSpecificity
+      unless itemSpecificity < ItemSpecificities.get(matchingItem)
+        menu[matchingItemIndex] = item
   else
     menu.push(item)