Restore original context menu ordering

Previously I used CSS specificity to order the most specific / recently
added menu items for a given element *first* when building up the
context menu. When a duplicate label was found for a given menu I would
refrain from inserting it. Now instead I order things the opposite way.
The most specific / recently added items come later and items with the
same label are clobbered by later items.

spec/context-menu-manager-spec.coffee

@@ -43,8 +43,8 @@ describe "ContextMenuManager", ->
       expect(contextMenu.templateForElement(grandchild)).toEqual [{
         label: 'A',
         submenu: [
-          {label: 'C', command: 'c'}
           {label: 'B', command: 'b'}
+          {label: 'C', command: 'c'}
         ]
       }]
 

spec/package-manager-spec.coffee

@@ -237,9 +237,9 @@ describe "PackageManager", ->
             expect(atom.menu.template.length).toBe 2
             expect(atom.menu.template[0].label).toBe "Second to Last"
             expect(atom.menu.template[1].label).toBe "Last"
-            expect(atom.contextMenu.templateForElement(element)[0].label).toBe "Menu item 3"
+            expect(atom.contextMenu.templateForElement(element)[0].label).toBe "Menu item 1"
             expect(atom.contextMenu.templateForElement(element)[1].label).toBe "Menu item 2"
-            expect(atom.contextMenu.templateForElement(element)[2].label).toBe "Menu item 1"
+            expect(atom.contextMenu.templateForElement(element)[2].label).toBe "Menu item 3"
 
         describe "when the metadata contains a 'menus' manifest", ->
           it "loads only the menus specified by the manifest, in the specified order", ->
@@ -251,8 +251,8 @@ describe "PackageManager", ->
 
             expect(atom.menu.template[0].label).toBe "Second to Last"
             expect(atom.menu.template[1].label).toBe "Last"
-            expect(atom.contextMenu.templateForElement(element)[0].label).toBe "Menu item 1"
-            expect(atom.contextMenu.templateForElement(element)[1].label).toBe "Menu item 2"
+            expect(atom.contextMenu.templateForElement(element)[0].label).toBe "Menu item 2"
+            expect(atom.contextMenu.templateForElement(element)[1].label).toBe "Menu item 1"
             expect(atom.contextMenu.templateForElement(element)[2]).toBeUndefined()
 
       describe "stylesheet loading", ->

src/context-menu-manager.coffee

@@ -97,7 +97,9 @@ class ContextMenuManager
 
     new Disposable =>
       for itemSet in addedItemSets
+        console.log "removing", itemSet, @itemSets.indexOf(itemSet)
         @itemSets.splice(@itemSets.indexOf(itemSet), 1)
+        console.log "remaining", @itemSets.slice()
 
   templateForElement: (target) ->
     @templateForEvent({target})
@@ -119,8 +121,7 @@ class ContextMenuManager
           if typeof item.shouldDisplay is 'function'
             continue unless item.shouldDisplay(event)
           item.created?(event)
-          templateItem = _.pick(item, 'type', 'label', 'command', 'submenu', 'commandOptions')
-          MenuHelpers.merge(template, templateItem)
+          MenuHelpers.merge(template, MenuHelpers.cloneMenuItem(item))
 
       currentTarget = currentTarget.parentElement
 
@@ -170,6 +171,7 @@ class ContextMenuItemSet
     @specificity = (SpecificityCache[@selector] ?= specificity(@selector))
     @sequenceNumber = SequenceCount++
 
+  # more specific / recent item sets sort later, because we clobber existing menu items
   compare: (other) ->
-    other.specificity - @specificity  or
-      other.sequenceNumber - @sequenceNumber
+    @specificity - other.specificity or
+      @sequenceNumber - other.sequenceNumber

src/menu-helpers.coffee

@@ -1,26 +1,34 @@
+_ = require 'underscore-plus'
+
 merge = (menu, item) ->
-  matchingItem = findMatchingItem(menu, item)
+  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
   else
     menu.push(item)
 
 unmerge = (menu, item) ->
-  if matchingItem = findMatchingItem(menu, item)
+  matchingItemIndex = findMatchingItemIndex(menu, item)
+  matchingItem = menu[matchingItemIndex] unless matchingItemIndex is - 1
+
+  if matchingItem?
     if item.submenu?
       unmerge(matchingItem.submenu, submenuItem) for submenuItem in item.submenu
 
     unless matchingItem.submenu?.length > 0
-      menu.splice(menu.indexOf(matchingItem), 1)
+      menu.splice(matchingItemIndex, 1)
 
-findMatchingItem = (menu, {type, label, submenu}) ->
-  return if type is 'separator'
-  for item in menu
+findMatchingItemIndex = (menu, {type, label, submenu}) ->
+  return -1 if type is 'separator'
+  for item, index in menu
     if normalizeLabel(item.label) is normalizeLabel(label) and item.submenu? is submenu?
-      return item
-  return
+      return index
+  -1
 
 normalizeLabel = (label) ->
   return undefined unless label?
@@ -30,4 +38,11 @@ normalizeLabel = (label) ->
   else
     label.replace(/\&/g, '')
 
-module.exports = {merge, unmerge, findMatchingItem, normalizeLabel}
+
+cloneMenuItem = (item) ->
+  item = _.pick(item, 'type', 'label', 'command', 'submenu', 'commandOptions')
+  if item.submenu?
+    item.submenu = item.submenu.map (submenuItem) -> cloneMenuItem(submenuItem)
+  item
+
+module.exports = {merge, unmerge, normalizeLabel, cloneMenuItem}