From 4abcb0fd331dd8ffac87abdcb8df104c73282275 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sun, 28 Dec 2025 19:17:54 +0000
Subject: [PATCH] Address code review feedback: improve efficiency and
 documentation

Co-authored-by: Kinneyzhang <38454496+Kinneyzhang@users.noreply.github.com>
---
 tp-tests.el |  7 ++++---
 tp.el       | 20 ++++++++++++++------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/tp-tests.el b/tp-tests.el
index 67ab73d..a47b20c 100644
--- a/tp-tests.el
+++ b/tp-tests.el
@@ -3506,6 +3506,7 @@ When using tp-set (direct property setting), tp-name is NOT added."
     (define-tp tp-space (pixel)
       `(display (space :width (,pixel))))
     ;; Define the button widget similar to the example
+    ;; Note: Using tp-button as the property name to match problem statement
     (tp-define-twidget button
       :props '(action (bgcolor . "green"))
       :slot 'label
@@ -3516,7 +3517,7 @@ When using tp-set (direct property setting), tp-name is NOT added."
                                   (tp-set " " 'tp-space 2)
                                   slot (tp-set " " 'tp-space 2))
                           'face `(:background ,bgcolor)
-                          'button-action `(:action ,action)))))
+                          'tp-button `(:action ,action)))))
     ;; Parse the button widget
     (let ((result (tp-widget-parse
                    '(button :action (lambda ()
@@ -3527,8 +3528,8 @@ When using tp-set (direct property setting), tp-name is NOT added."
       ;; Check that the face property is applied
       (let ((face (get-text-property 2 'face result)))
         (should (equal (plist-get face :background) "green")))
-      ;; Check that the button-action property is applied
-      (let ((action-prop (get-text-property 2 'button-action result)))
+      ;; Check that the tp-button property is applied (matches problem statement)
+      (let ((action-prop (get-text-property 2 'tp-button result)))
         (should (listp (plist-get action-prop :action)))))))
 
 (ert-deftest tp-test-twidget-reset ()
diff --git a/tp.el b/tp.el
index 85e5521..a166784 100644
--- a/tp.el
+++ b/tp.el
@@ -3586,9 +3586,10 @@ Example:
 PROPS is a list of property definitions.
 SLOT is the slot name symbol.
 RENDER is the render function."
-  (let ((definition (list :props props :slot slot :render render)))
-    (if (assoc name tp-twidget-alist)
-        (setf (cdr (assoc name tp-twidget-alist)) definition)
+  (let ((definition (list :props props :slot slot :render render))
+        (existing (assoc name tp-twidget-alist)))
+    (if existing
+        (setcdr existing definition)
       (push (cons name definition) tp-twidget-alist)))
   (assoc name tp-twidget-alist))
 
@@ -3614,7 +3615,12 @@ Returns nil if no default is specified."
   "Parse and render a widget invocation.
 
 WIDGET-FORM is a list starting with the widget name, followed by
-keyword-value pairs for props, and ending with the slot value.
+keyword-value pairs for props, and ending with a single slot value.
+
+The format is: (WIDGET-NAME :prop1 val1 :prop2 val2 ... SLOT-VALUE)
+
+Keyword arguments must come before the slot value. The slot value
+is the last non-keyword argument and must be exactly one value.
 
 Example:
   (tp-widget-parse
@@ -3646,9 +3652,11 @@ Returns the rendered string with text properties applied."
                 (val (cadr args)))
             (push (cons key val) collected-props)
             (setq args (cddr args))))
-        ;; The remaining argument is the slot value
+        ;; The remaining argument(s) should be the slot value (exactly one)
         (when args
-          (setq slot-value (car args)))
+          (setq slot-value (car args))
+          (when (cdr args)
+            (warn "tp-widget-parse: Extra arguments after slot value ignored: %S" (cdr args))))
         ;; Build the props plist with defaults
         (dolist (prop-def prop-defs)
           (let* ((prop-name (tp--twidget-prop-name prop-def))