core: add layer shadowing system

Layers can now declare in their layers.el file that they shadow one or more
layers using the following functions:

- configuration-layer/shadow-layers
- configuration-layer/shadow-layer

Those function are commutative so:
    (configuration-layer/shadow-layer 'layer1 'layer2)
is the same as
    (configuration-layer/shadow-layer 'layer2 'layer1)
and means that
layer1 shadows layer2
and
layer2 shadows layer1

The typical use-case is helm and ivy layers. Helm shadows the ivy layer and
Ivy shadows the helm layer.

Shadowing is sensitive to the order of declaration of layers in the dotfile,
for instance:

    (setq dotspacemacs-configuration-layers '(
      helm
      ivy
     ))

means that ivy shadows helm so helm layer is effectively ignored,
whereas

    (setq dotspacemacs-configuration-layers '(
      ivy
      helm
     ))

means that helm shadows ivy so ivy layer is effectively ignored.

This mechanism can be turned off using the :can-shadow keyword:

    (setq dotspacemacs-configuration-layers '(
      ivy
      (helm :can-shadow nil)
     ))

means that both ivy and helm layers will be installed (not recommended in this
case)

Note that the `:can-shadow` mechanism will be fully implemented in a next
commit.

core/core-configuration-layer.el

@@ -116,7 +116,15 @@ ROOT is returned."
             :initform 'unspecified
             :type (satisfies (lambda (x) (or (listp x) (eq 'unspecified x))))
             :documentation
-            "A list of layers where this layer is enabled. (Takes precedence over `:disabled-for'.)"))
+            "A list of layers where this layer is enabled. (Takes precedence over `:disabled-for'.)")
+   (can-shadow :initarg :can-shadow
+               :initform t
+               :type boolean
+               :documentation "If non-nil this layer can shadow other layers.")
+   (shadowed-by :initarg :shadowed-by
+                :initform nil
+                :type list
+                :documentation "A list of layers that can shadow this layer."))
   "A configuration layer.")
 
 (defmethod cfgl-layer-owned-packages ((layer cfgl-layer) &optional props)
@@ -134,9 +142,25 @@ LAYER has to be installed for this method to work properly."
   "Accept nil as argument and return nil."
   nil)
 
+(defmethod cfgl-layer-shadowed-p ((layer cfgl-layer))
+  "Return the list of layers that shadow LAYER."
+  (let ((rank (cl-position (oref layer :name) configuration-layer--used-layers))
+        shadowing-layers)
+    (when (numberp rank)
+      (mapcar
+       (lambda (other)
+         (let ((orank (cl-position other configuration-layer--used-layers)))
+           ;; LAYER is shadowed by OTHER if and only if its rank is lower than
+           ;; OTHER's rank.
+           (when (and (numberp orank) (< rank orank))
+             (add-to-list 'shadowing-layers other))))
+       (oref layer :shadowed-by)))
+    shadowing-layers))
+
 (defmethod cfgl-layer-get-packages ((layer cfgl-layer) &optional props)
   "Return the list of packages for LAYER.
-If PROPS is non-nil then return packages as lists with their properties"
+If PROPS is non-nil then return packages as lists along with their properties.
+Returns nil if the layer is shadowed by a layer."
   (let ((all (eq 'all (oref layer :selected-packages))))
     (delq nil (mapcar
                (lambda (x)
@@ -498,7 +522,7 @@ To prevent package from being installed or uninstalled set the variable
   ;; configure used packages
   (configuration-layer//configure-packages configuration-layer--used-packages)
   (configuration-layer//load-layers-files configuration-layer--used-layers
-                         '("keybindings.el"))
+                                          '("keybindings.el"))
   (run-hooks 'configuration-layer-post-load-hook))
 
 (defun configuration-layer/load-auto-layer-file ()
@@ -602,6 +626,11 @@ If USEDP or `configuration-layer--load-packages-files' is non-nil then the
                         'unspecified))
              (variables (when (listp layer-specs)
                           (spacemacs/mplist-get layer-specs :variables)))
+             (can-shadow
+              (if (and (listp layer-specs)
+                       (memq :can-shadow layer-specs))
+                  (nth 0 (spacemacs/mplist-get layer-specs :can-shadow))
+                'unspecified))
              (packages-file (concat dir "packages.el"))
              (packages
               (if (and (or usedp configuration-layer--load-packages-files)
@@ -619,7 +648,9 @@ If USEDP or `configuration-layer--load-packages-files' is non-nil then the
         (when usedp
           (oset obj :disabled-for disabled)
           (oset obj :enabled-for enabled)
-          (oset obj :variables variables))
+          (oset obj :variables variables)
+          (unless (eq 'unspecified can-shadow)
+            (oset obj :can-shadow can-shadow)))
         (when packages
           (oset obj :packages packages)
           (oset obj :selected-packages selected-packages))
@@ -1016,13 +1047,18 @@ If SKIP-LAYER-DISCOVERY is non-nil then do not check for new layers."
   "Read the package lists of layers with name LAYER-NAMES and create packages.
 USEDP if non-nil indicates that made packages are used packages."
   (dolist (layer-name layer-names)
-    (let ((layer (configuration-layer/get-layer layer-name)))
+    (let* ((layer (configuration-layer/get-layer layer-name))
-      (dolist (pkg (cfgl-layer-get-packages layer 'with-props))
+           (shadowed-by (cfgl-layer-shadowed-p layer)))
-        (let* ((pkg-name (if (listp pkg) (car pkg) pkg))
+      (if shadowed-by
-               (obj (configuration-layer/get-package pkg-name)))
+          (spacemacs-buffer/message
-          (setq obj (configuration-layer/make-package pkg layer-name obj))
+           "Ignoring layer '%s' because it is shadowed by layer(s) '%s'."
-          (configuration-layer//add-package
+           layer-name shadowed-by)
-           obj (and (cfgl-package-get-safe-owner obj) usedp)))))))
+        (dolist (pkg (cfgl-layer-get-packages layer 'with-props))
+          (let* ((pkg-name (if (listp pkg) (car pkg) pkg))
+                 (obj (configuration-layer/get-package pkg-name)))
+            (setq obj (configuration-layer/make-package pkg layer-name obj))
+            (configuration-layer//add-package
+             obj (and (cfgl-package-get-safe-owner obj) usedp))))))))
 
 (defun configuration-layer/make-packages-from-dotfile (&optional usedp)
   "Read the additonal packages declared in the dotfile and create packages.
@@ -1286,8 +1322,7 @@ wether the declared layer is an used one or not."
           (configuration-layer//set-layer-variables obj)
           (when (or usedp configuration-layer--load-packages-files)
             (configuration-layer//load-layer-files layer-name '("layers.el"))))
-      (configuration-layer//warning "Unknown layer %s declared in dotfile."
+      (configuration-layer//warning "Unknown declared layer %s." layer-name))))
-                                    layer-name))))
 
 (defun configuration-layer//declare-used-layers (layers-specs)
   "Declare used layers from LAYERS-SPECS list."
@@ -1315,6 +1350,33 @@ wether the declared layer is an used one or not."
         (configuration-layer/declare-layer distribution)))
     (configuration-layer/declare-layer 'spacemacs-bootstrap)))
 
+(defun configuration-layer/shadow-layers (layer-name shadowed-layers)
+  "Declare LAYER-NAME to shadow SHADOWED-LAYERS.
+LAYER-NAME is a the name symbol of an existing layer.
+SHADOWED-LAYERS is a list of layer name symbols."
+  (mapc (lambda (x)
+          (configuration-layer/shadow-layer layer-name x))
+        shadowed-layers))
+
+(defun configuration-layer/shadow-layer (layer-name shadowed-layer-name)
+  "Declare LAYER-NAME to shadow SHADOWED-LAYER.
+LAYER-NAME is a the name symbol of an existing layer.
+SHADOWED-LAYER-NAME is the name symbol of an existing layer."
+  (let* ((layer (configuration-layer/get-layer layer-name))
+         (shadowed-layer (configuration-layer/get-layer shadowed-layer-name)))
+    (if (and layer shadowed-layer)
+        (progn
+          ;; note: shadowing is commutative
+          (cl-pushnew layer-name (oref shadowed-layer :shadowed-by))
+          (cl-pushnew shadowed-layer-name (oref layer :shadowed-by)))
+      ;; cannot find one or both layers
+      (if (null layer)
+          (configuration-layer//warning "Unknown layer %s to shadow %s."
+                                        layer-name shadowed-layer-name))
+      (if (null shadowed-layer)
+          (configuration-layer//warning "Unknown shadowed layer %s by %s."
+                                        shadowed-layer-name layer-name)))))
+
 (defun configuration-layer//set-layers-variables (layers)
   "Set the configuration variables for the passed LAYERS."
   (mapc 'configuration-layer//set-layer-variables layers))
@@ -1338,10 +1400,11 @@ wether the declared layer is an used one or not."
                                     var))))))
 
 (defun configuration-layer/layer-used-p (layer-name)
-  "Return non-nil if LAYER-NAME is the name of a used layer."
+  "Return non-nil if LAYER-NAME is the name of a used and non-shadowed layer."
   (or (eq 'dotfile layer-name)
       (let ((obj (configuration-layer/get-layer layer-name)))
-        (when obj (memq layer-name configuration-layer--used-layers)))))
+        (when obj (and (not (cfgl-layer-shadowed-p obj))
+                   (memq layer-name configuration-layer--used-layers))))))
 (defalias 'configuration-layer/layer-usedp
   'configuration-layer/layer-used-p)
 

layers/+completion/helm/layers.el

@@ -0,0 +1,12 @@
+;;; layers.el --- Helm layer layers File for Spacemacs
+;;
+;; Copyright (c) 2012-2017 Sylvain Benner & Contributors
+;;
+;; Author: Sylvain Benner <sylvain.benner@gmail.com>
+;; URL: https://github.com/syl20bnr/spacemacs
+;;
+;; This file is not part of GNU Emacs.
+;;
+;;; License: GPLv3
+
+(configuration-layer/shadow-layer 'helm 'ivy)

layers/+completion/ivy/layers.el

@@ -9,6 +9,8 @@
 ;;
 ;;; License: GPLv3
 
+(configuration-layer/shadow-layer 'ivy 'helm)
+
 ;; smex is handled by the `ivy' layer and we don't want
 ;; to use the ownership mechanism of layers because it is dependent
 ;; on the order of layer declaration

layers/+distributions/spacemacs/layers.el

@@ -24,14 +24,3 @@
                                       spacemacs-purpose
                                       spacemacs-visual
                                       ))
-;; If the user has not explicitly declared `helm' or `ivy'
-;; and they are using the standard distribution, assume they
-;; want `helm' completion.
-(unless (or (configuration-layer/layer-used-p 'ivy)
-            (configuration-layer/layer-used-p 'helm))
-  (configuration-layer/declare-layers '(helm)))
-
-(when (and (configuration-layer/layer-used-p 'ivy)
-           (configuration-layer/layer-used-p 'helm))
-  (spacemacs-buffer/warning (concat "Both the `helm' and `ivy' layers are enabled. "
-                             "This may lead to unexpected behaviour.")))

tests/core/core-configuration-layer-utest.el

@@ -15,7 +15,13 @@
 (defun helper--add-layers (layers &optional usedp)
   "Set the layer variables given a list of LAYERS objects."
   (dolist (layer layers)
-    (configuration-layer//add-layer layer usedp)))
+    (configuration-layer//add-layer layer usedp))
+  ;; hackish but we need to reverse the list in order to have the layer
+  ;; in the correct order (this reverse in normally performed in function
+  ;; configuration-layer//declare-used-layers )
+  (when usedp
+    (setq configuration-layer--used-layers
+          (reverse configuration-layer--used-layers))))
 
 (defun helper--add-packages (packages &optional usedp)
   "Set the package variables given a list of PACKAGES objects."
@@ -164,6 +170,30 @@
                            :selected-packages '(pkg-unknown))))
     (should (null (cfgl-layer-get-packages layer)))))
 
+;; method: cfgl-layer-shadowed-p
+
+(ert-deftest test-cfgl-layer-shadowed-p--layer2-shadows-layer1 ()
+  (let ((layer1 (cfgl-layer "layer1" :name 'layer1))
+        (layer2 (cfgl-layer "layer2" :name 'layer2))
+        (configuration-layer--used-layers nil)
+        (configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (helper--add-layers `(,layer1 ,layer2) 'used)
+    (configuration-layer/shadow-layer 'layer2 'layer1)
+    (should (and (equal '(layer2) (cfgl-layer-shadowed-p layer1))
+                 (not (cfgl-layer-shadowed-p layer2))))))
+
+(ert-deftest test-cfgl-layer-shadowed-p--layer1-shadows-layer2 ()
+  (let ((layer1 (cfgl-layer "layer1" :name 'layer1))
+        (layer2 (cfgl-layer "layer2" :name 'layer2))
+        (configuration-layer--used-layers nil)
+        (configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    ;; we just switched the order of used layers
+    ;; remember, shadowing is commutative
+    (helper--add-layers `(,layer2 ,layer1) 'used)
+    (configuration-layer/shadow-layer 'layer2 'layer1)
+    (should (and (equal '(layer1) (cfgl-layer-shadowed-p layer2))
+                 (not (cfgl-layer-shadowed-p layer1))))))
+
 ;; ---------------------------------------------------------------------------
 ;; configuration-layer/layer-used-p
 ;; ---------------------------------------------------------------------------
@@ -182,6 +212,15 @@
     (helper--add-layers `(,(cfgl-layer "notusedlayer" :name 'notusedlayer)))
     (should (null (configuration-layer/layer-used-p 'notusedlayer)))))
 
+(ert-deftest test-layer-used-p--returns-false-when-layer-is-shadowed ()
+  (let ((usedlayer1 (cfgl-layer "usedlayer1" :name 'usedlayer1))
+        (usedlayer2 (cfgl-layer "usedlayer2" :name 'usedlayer2))
+        configuration-layer--used-layers
+        (configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (helper--add-layers `(,usedlayer1 ,usedlayer2) 'used)
+    (configuration-layer/shadow-layer 'usedlayer2 'usedlayer1)
+    (should (not (configuration-layer/layer-used-p 'usedlayer1)))))
+
 (ert-deftest test-layer-used-p--dotfile-layer-is-always-used ()
   (should (configuration-layer/layer-used-p 'dotfile)))
 
@@ -815,6 +854,8 @@
 ;; configuration-layer/make-layer
 ;; ---------------------------------------------------------------------------
 
+;; layer directory
+
 (ert-deftest test-make-layer--make-layer-from-symbol-with-a-dir ()
   (should (equal (cfgl-layer "layer"
                              :name 'layer
@@ -852,6 +893,8 @@
                                :dir spacemacs-start-directory)
                    (configuration-layer/make-layer 'layer layer)))))
 
+;; load packages
+
 (ert-deftest test-make-layer--make-used-layer-loads-packages-file ()
   (let ((layer (cfgl-layer "layer"
                            :name 'layer
@@ -899,11 +942,14 @@
      ((file-exists-p (f) ((:output t :occur 1))))
      (configuration-layer/make-layer 'layer layer))))
 
+;; set/override properties
+
 (ert-deftest test-make-layer--make-used-layer-can-set-additional-properties ()
   (let ((layer (cfgl-layer "layer"
                            :name 'layer
                            :dir spacemacs-start-directory))
         (layer-specs '(layer :disabled-for pkg8 pkg9
+                             :can-shadow nil
                              :variables foo bar toto 1))
         (layer-packages '(pkg1 pkg2 pkg3))
         (mocker-mock-default-record-cls 'mocker-stub-record))
@@ -913,6 +959,7 @@
      (should (equal (cfgl-layer "layer"
                                 :name 'layer
                                 :disabled-for '(pkg8 pkg9)
+                                :can-shadow nil
                                 :variables '(foo bar toto 1)
                                 :packages '(pkg1 pkg2 pkg3)
                                 :selected-packages 'all
@@ -924,11 +971,13 @@
                            :name 'layer
                            :dir spacemacs-start-directory))
         (layer-specs '(layer :disabled-for pkg8 pkg9
+                             :can-shadow nil
                              :variables foo bar toto 1))
         (layer-packages '(pkg1 pkg2 pkg3)))
     (should (equal (cfgl-layer "layer"
                                :name 'layer
                                :disabled-for nil
+                               :can-shadow t
                                :variables nil
                                :packages nil
                                :selected-packages 'all
@@ -939,9 +988,11 @@
   (let ((layer (cfgl-layer "layer"
                            :name 'layer
                            :disabled-for '(pkg10)
+                           :can-shadow nil
                            :variables '(titi tata tutu 1)
                            :dir spacemacs-start-directory))
         (layer-specs '(layer :disabled-for pkg8 pkg9
+                             :can-shadow t
                              :variables foo bar toto 1))
         (layer-packages '(pkg1 pkg2 pkg3))
         (mocker-mock-default-record-cls 'mocker-stub-record))
@@ -951,6 +1002,7 @@
      (should (equal (cfgl-layer "layer"
                                 :name 'layer
                                 :disabled-for '(pkg8 pkg9)
+                                :can-shadow t
                                 :variables '(foo bar toto 1)
                                 :packages '(pkg1 pkg2 pkg3)
                                 :selected-packages 'all
@@ -961,22 +1013,144 @@
   (let ((layer (cfgl-layer "layer"
                            :name 'layer
                            :disabled-for '(pkg10)
+                           :can-shadow nil
                            :variables '(titi tata tutu 1)
                            :packages '(pkg1 pkg2 pkg3)
                            :selected-packages 'all
                            :dir spacemacs-start-directory))
         (layer-specs '(layer :disabled-for pkg8 pkg9
+                             :can-shadow t
                              :variables foo bar toto 1))
         (mocker-mock-default-record-cls 'mocker-stub-record))
     (should (equal (cfgl-layer "layer"
                                :name 'layer
                                :disabled-for '(pkg10)
+                               :can-shadow nil
                                :variables '(titi tata tutu 1)
                                :packages '(pkg1 pkg2 pkg3)
                                :selected-packages 'all
                                :dir spacemacs-start-directory)
                    (configuration-layer/make-layer layer-specs layer)))))
 
+;; shadow layers
+
+(ert-deftest test-make-layer--by-default-layer-can-shadow-other-layers ()
+  (let ((layer-specs 'layer)
+        (mocker-mock-default-record-cls 'mocker-stub-record))
+    (should
+     (equal (cfgl-layer "layer"
+                        :name 'layer
+                        :dir spacemacs-start-directory
+                        :can-shadow t)
+            (configuration-layer/make-layer layer-specs nil nil
+                                            spacemacs-start-directory)))))
+
+(ert-deftest test-make-layer--force-used-layer-to-not-shadow-other-layers ()
+  (let ((layer-specs '(layer :can-shadow nil))
+        (mocker-mock-default-record-cls 'mocker-stub-record))
+    (should
+     (equal (cfgl-layer "layer"
+                        :name 'layer
+                        :dir spacemacs-start-directory
+                        :can-shadow nil)
+            (configuration-layer/make-layer layer-specs nil 'used
+                                            spacemacs-start-directory)))))
+(ert-deftest test-make-layer--unused-layer-can-always-shadow-other-layers ()
+  (let ((layer-specs '(layer :can-shadow nil))
+        (mocker-mock-default-record-cls 'mocker-stub-record))
+    (should
+     (equal (cfgl-layer "layer"
+                        :name 'layer
+                        :dir spacemacs-start-directory
+                        :can-shadow t)
+            (configuration-layer/make-layer layer-specs nil nil
+                                            spacemacs-start-directory)))))
+
+;; ---------------------------------------------------------------------------
+;; configuration-layer//shadow-layer
+;; ---------------------------------------------------------------------------
+
+(ert-deftest test-shadow-layer--layer-1-shadows-layer2 ()
+  (let ((configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (helper--add-layers
+     `(,(cfgl-layer "layer-shadow-1" :name 'layer-shadow-1)
+       ,(cfgl-layer "layer-shadow-2" :name 'layer-shadow-2)))
+    (configuration-layer/shadow-layer 'layer-shadow-1 'layer-shadow-2)
+    (should (equal '(layer-shadow-1)
+                   (oref (configuration-layer/get-layer 'layer-shadow-2)
+                         :shadowed-by)))))
+
+(ert-deftest test-shadow-layer--layer-2-shadows-layer1-as-well--commutativity ()
+  (let ((configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (helper--add-layers
+     `(,(cfgl-layer "layer-shadow-1" :name 'layer-shadow-1)
+       ,(cfgl-layer "layer-shadow-2" :name 'layer-shadow-2)))
+    (configuration-layer/shadow-layer 'layer-shadow-1 'layer-shadow-2)
+    (should (equal '(layer-shadow-2)
+                   (oref (configuration-layer/get-layer 'layer-shadow-1)
+                         :shadowed-by)))))
+
+(ert-deftest test-shadow-layer--idempotency-and-commutatitivity ()
+  (let ((configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (helper--add-layers
+     `(,(cfgl-layer "layer-shadow-1" :name 'layer-shadow-1)
+       ,(cfgl-layer "layer-shadow-2" :name 'layer-shadow-2)))
+    (dotimes (i 2)
+     (configuration-layer/shadow-layer 'layer-shadow-1 'layer-shadow-2))
+    (dotimes (i 2)
+      (configuration-layer/shadow-layer 'layer-shadow-2 'layer-shadow-1))
+    (should (and (equal '(layer-shadow-2)
+                        (oref (configuration-layer/get-layer 'layer-shadow-1)
+                              :shadowed-by))
+                 (equal '(layer-shadow-1)
+                        (oref (configuration-layer/get-layer 'layer-shadow-2)
+                              :shadowed-by))))))
+
+(ert-deftest test-shadow-layer--layer-1-shadows-multiple-layers ()
+  (let ((configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (helper--add-layers
+     `(,(cfgl-layer "layer-shadow-1" :name 'layer-shadow-1)
+       ,(cfgl-layer "layer-shadow-2" :name 'layer-shadow-2)
+       ,(cfgl-layer "layer-shadow-3" :name 'layer-shadow-3)))
+    (configuration-layer/shadow-layer 'layer-shadow-1 'layer-shadow-2)
+    (configuration-layer/shadow-layer 'layer-shadow-1 'layer-shadow-3)
+    (should (and (equal '(layer-shadow-1)
+                        (oref (configuration-layer/get-layer 'layer-shadow-2)
+                              :shadowed-by))
+                 (equal '(layer-shadow-1)
+                        (oref (configuration-layer/get-layer 'layer-shadow-3)
+                              :shadowed-by))))))
+
+(ert-deftest test-shadow-layer--unknown-layer-shadows-existing-layer ()
+  (let ((configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (helper--add-layers
+     `(,(cfgl-layer "layer-shadow-2" :name 'layer-shadow-2)))
+    (mocker-let
+     ((configuration-layer//warning
+       (msg &rest args)
+       ((:record-cls 'mocker-stub-record :output nil :occur 1))))
+     (configuration-layer/shadow-layer 'layer-shadow-1 'layer-shadow-2)
+     (should (null (oref (configuration-layer/get-layer 'layer-shadow-2)
+                         :shadowed-by))))))
+
+(ert-deftest test-shadow-layer--existing-layer-shadows-non-existing-layer ()
+  (let ((configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (helper--add-layers
+     `(,(cfgl-layer "layer-shadow-1" :name 'layer-shadow-1)))
+    (mocker-let
+     ((configuration-layer//warning
+       (msg &rest args)
+       ((:record-cls 'mocker-stub-record :output nil :occur 1))))
+     (configuration-layer/shadow-layer 'layer-shadow-1 'layer-shadow-2))))
+
+(ert-deftest test-shadow-layer--unknown-layer-shadows-unknown-layer ()
+  (let ((configuration-layer--indexed-layers (make-hash-table :size 1024)))
+    (mocker-let
+     ((configuration-layer//warning
+       (msg &rest args)
+       ((:record-cls 'mocker-stub-record :output nil :occur 2))))
+     (configuration-layer/shadow-layer 'layer-shadow-1 'layer-shadow-2))))
+
 ;; ---------------------------------------------------------------------------
 ;; configuration-layer//set-layers-variables
 ;; ---------------------------------------------------------------------------