Address code review feedback: improve format validation and fix index increment logic
Co-authored-by: Kinneyzhang <38454496+Kinneyzhang@users.noreply.github.com>
This commit is contained in:
parent
b6f3f87925
commit
ff96cf25af
71
tp.el
71
tp.el
@ -1546,6 +1546,31 @@ The layer is stored in `tp-layer-alist'."
|
|||||||
(push (cons ',name ',properties) tp-layer-alist))
|
(push (cons ',name ',properties) tp-layer-alist))
|
||||||
(assoc ',name tp-layer-alist))))
|
(assoc ',name tp-layer-alist))))
|
||||||
|
|
||||||
|
(defun tp--layer-group-element-format (element)
|
||||||
|
"Determine the format type of ELEMENT.
|
||||||
|
Returns 'symbol, 'format-1, 'format-2, 'format-3, or nil if invalid."
|
||||||
|
(cond
|
||||||
|
;; Symbol - reference to existing layer
|
||||||
|
((symbolp element) 'symbol)
|
||||||
|
;; Format 3 - ("name" :props (plist...))
|
||||||
|
((and (listp element)
|
||||||
|
(= (length element) 3)
|
||||||
|
(stringp (car element))
|
||||||
|
(eq (cadr element) :props)
|
||||||
|
(listp (caddr element)))
|
||||||
|
'format-3)
|
||||||
|
;; Format 2 - ("name" . (plist...)) - cons cell with proper list cdr
|
||||||
|
((and (consp element)
|
||||||
|
(stringp (car element))
|
||||||
|
(listp (cdr element))
|
||||||
|
(not (eq (cadr element) :props))) ; Distinguish from format-3
|
||||||
|
'format-2)
|
||||||
|
;; Format 1 - (plist...) - anonymous, must start with a symbol
|
||||||
|
((and (listp element)
|
||||||
|
(symbolp (car element)))
|
||||||
|
'format-1)
|
||||||
|
(t nil)))
|
||||||
|
|
||||||
(defun tp--parse-layer-group-element (group-name element idx)
|
(defun tp--parse-layer-group-element (group-name element idx)
|
||||||
"Parse a layer group element and return (layer-name . properties).
|
"Parse a layer group element and return (layer-name . properties).
|
||||||
GROUP-NAME is the name of the layer group.
|
GROUP-NAME is the name of the layer group.
|
||||||
@ -1554,33 +1579,23 @@ IDX is the index for anonymous elements.
|
|||||||
|
|
||||||
Returns a cons cell (LAYER-NAME . PROPERTIES) or a symbol if ELEMENT
|
Returns a cons cell (LAYER-NAME . PROPERTIES) or a symbol if ELEMENT
|
||||||
references an already-defined layer."
|
references an already-defined layer."
|
||||||
(cond
|
(let ((format (tp--layer-group-element-format element)))
|
||||||
;; Case: already defined layer (symbol)
|
(pcase format
|
||||||
((symbolp element)
|
('symbol element)
|
||||||
element)
|
('format-3
|
||||||
;; Case: Format 3 - ("name" :props (plist...))
|
(let* ((layer-suffix (car element))
|
||||||
((and (listp element)
|
(layer-name (intern (format "%s-%s" group-name layer-suffix)))
|
||||||
(stringp (car element))
|
(props (caddr element)))
|
||||||
(eq (cadr element) :props)
|
(cons layer-name props)))
|
||||||
(caddr element))
|
('format-2
|
||||||
(let* ((layer-suffix (car element))
|
(let* ((layer-suffix (car element))
|
||||||
(layer-name (intern (format "%s-%s" group-name layer-suffix)))
|
(layer-name (intern (format "%s-%s" group-name layer-suffix)))
|
||||||
(props (caddr element)))
|
(props (cdr element)))
|
||||||
(cons layer-name props)))
|
(cons layer-name props)))
|
||||||
;; Case: Format 2 - ("name" . (plist...)) - cons cell
|
('format-1
|
||||||
((and (consp element)
|
(let ((layer-name (intern (format "%s-%d" group-name idx))))
|
||||||
(stringp (car element))
|
(cons layer-name element)))
|
||||||
(listp (cdr element)))
|
(_ (error "Invalid layer group element: %S" element)))))
|
||||||
(let* ((layer-suffix (car element))
|
|
||||||
(layer-name (intern (format "%s-%s" group-name layer-suffix)))
|
|
||||||
(props (cdr element)))
|
|
||||||
(cons layer-name props)))
|
|
||||||
;; Case: Format 1 - (plist...) - anonymous, use index
|
|
||||||
((and (listp element)
|
|
||||||
(not (stringp (car element))))
|
|
||||||
(let ((layer-name (intern (format "%s-%d" group-name idx))))
|
|
||||||
(cons layer-name element)))
|
|
||||||
(t (error "Invalid layer group element: %S" element))))
|
|
||||||
|
|
||||||
(defmacro tp-define-layer-group (name &rest elements)
|
(defmacro tp-define-layer-group (name &rest elements)
|
||||||
"Define a layer group named NAME containing multiple layers.
|
"Define a layer group named NAME containing multiple layers.
|
||||||
@ -1634,7 +1649,7 @@ and the group itself is stored in `tp-layer-groups'."
|
|||||||
layer-defs)
|
layer-defs)
|
||||||
(push layer-name layer-names)
|
(push layer-name layer-names)
|
||||||
;; Only increment idx for anonymous (Format 1) elements
|
;; Only increment idx for anonymous (Format 1) elements
|
||||||
(unless (and (listp element) (stringp (car element)))
|
(when (eq (tp--layer-group-element-format element) 'format-1)
|
||||||
(cl-incf idx)))))))
|
(cl-incf idx)))))))
|
||||||
(setq layer-names (nreverse layer-names))
|
(setq layer-names (nreverse layer-names))
|
||||||
(setq layer-defs (nreverse layer-defs))
|
(setq layer-defs (nreverse layer-defs))
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user