From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Naoya Yamashita Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Add gv-define-expander for plist-get Date: Wed, 9 Sep 2020 10:03:20 +0900 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="00000000000095bc8905aed7064a" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="30645"; mail-complaints-to="usenet@ciao.gmane.io" Cc: "emacs-devel@gnu.org" To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Sep 09 03:04:41 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kFoXQ-0007r9-G5 for ged-emacs-devel@m.gmane-mx.org; Wed, 09 Sep 2020 03:04:40 +0200 Original-Received: from localhost ([::1]:37986 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kFoXP-0000zI-Af for ged-emacs-devel@m.gmane-mx.org; Tue, 08 Sep 2020 21:04:39 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:44962) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kFoWm-0000Yj-5y for emacs-devel@gnu.org; Tue, 08 Sep 2020 21:04:00 -0400 Original-Received: from mail-pg1-x542.google.com ([2607:f8b0:4864:20::542]:46300) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kFoWj-0007O7-Kw for emacs-devel@gnu.org; Tue, 08 Sep 2020 21:03:59 -0400 Original-Received: by mail-pg1-x542.google.com with SMTP id 34so777681pgo.13 for ; Tue, 08 Sep 2020 18:03:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=GdrmP/9iHh7uo+Ez/PvSjjQpnuKMiw20u8DaHGEcNw8=; b=dUw1rPifviFy7LhszAj9EmPg7xMLL1CIHJ4aOxVi6EsnpoguWnbhefFHuGZ9wfFKEM 1Dfyx34/5nM97skjPz9c/YLqar2l5kCfsROPbVvFI6PlVLRU3NAp+rOkFbCBsU+6nW1l zoukq+hG69zAznsP2vIpqRlhYqmXpmNYdkwBonD72ir+BNXQi/VZkyrnNYwZGUYVLPGV By5U2f70+U/oCVz8SJT/TEZLgYLvv5iR2h7SnxoGV/iTZlwhUnwTLRdfWHTGRgQDNMnN Y6IJKcUJB/VMNLq+ZuieN2E8MBZUwFU396+rdMfBli35aCeXiqIRso8u3LAEcBlIfrBU fx5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GdrmP/9iHh7uo+Ez/PvSjjQpnuKMiw20u8DaHGEcNw8=; b=HEzAr+hoQW82ZqRd+U6UOSPf7AYB3i/PoXvG1KW+Bwz60O+JSC3WBxcqBlUx7uuA9U MK7iFDIQfB2SvBwlDHjP3GvFjYqtgTi5DVOvAPrtSlgfFpPK82ECmXjgcLarSWDfWwBy +D3dv3PFC+Z+OYDmxDLkGr8WsPr/s+lSX3jYygqnOB0zotwyxWCK8G4qFnuBCyM+Zr3U d3XuKiKu1zTyY00VlI3kLMwQbZCKqAcmE+a6ViGgAo/sSsna4fP0I2hkMXeZXaan8S2Y sHGkGpZQuh2ukN7CjtrpaKzLG5+WlCzSS8hRPlvq15ef2e6l0v2OmEjpSA93GwjRKLHB XdKA== X-Gm-Message-State: AOAM533uxUIafyvCITAPnN0eTLHDVz2lt8xO09RNw3iWpraVrBH2tDBQ 4gyny2Sd9tUGZPXQqX7fJF+eOU1ACiZ/YvFkDUu6xOQzWGgDmA== X-Google-Smtp-Source: ABdhPJx2PzFsB3jmdW3HOE30ZLisVGVo7eGq7yps80ObcucsIxL7RRziKh8eBLHn2z7hg55pe39hmiflcIQB1Gl8PnE= X-Received: by 2002:a65:4d0c:: with SMTP id i12mr1044633pgt.267.1599613435198; Tue, 08 Sep 2020 18:03:55 -0700 (PDT) In-Reply-To: Received-SPF: pass client-ip=2607:f8b0:4864:20::542; envelope-from=conao3@gmail.com; helo=mail-pg1-x542.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -17 X-Spam_score: -1.8 X-Spam_bar: - X-Spam_report: (-1.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:254828 Archived-At: --00000000000095bc8905aed7064a Content-Type: multipart/alternative; boundary="00000000000095bc8405aed70648" --00000000000095bc8405aed70648 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > Sorry for not answering earlier. I thanks you for your great review! > The patch looks good (with special thanks for the tests), except that in > the code above, you shouldn't have those (repeated) `(plist-member > ,plist ,key)` since they not only waste time but they will also evaluate > `plist` multiple times which could change the semantics in case `plist` > has side effects. > I think these should refer to `p`, right? I think good too. > Furthermore, I think you need to use ',key rather than just ,key in case > the key is a cons or a plain symbol, so maybe you want to add a test > that uses symbols rather than keywords: > > ;; Other function (cl-rotatef) usage for plist-get. > (should (equal (let ((target '(a "a" b "b" c "c"))) > (cl-rotatef (plist-get target 'b) (plist-get target 'c)) > target) > '(a "a" b "c" c "b"))) I cannot understand this point. Your additional test is passed by current code, the additional quote is unneeded I think. If we add quote for `key`, the below code will be valid. (let ((target '(a "a" b "b" c "c"))) (setf (plist-get target b) "modify") target) ;;=3D> (a "a" b "modify" c "c") but this code will be invalid. (let ((target '(a "a" b "b" c "c"))) (setf (plist-get target 'b) "modify") target) ;;=3D> ('b "modify" a "a" b "b" c "c") I think the latter should be valid and the former should be invalid. This is an analogy from normal usage of plist-get. (let ((target '(a "a" b "b" c "c"))) (plist-get target b)) ;;=3D> Debugger entered--Lisp error: (void-variable b) ;; (plist-get target b) ;; (let ((target '(a "a" b "b" c "c"))) (plist-get target b)) ;; (progn (let ((target '(a "a" b "b" c "c"))) (plist-get target b))) ;; eval((progn (let ((target '(a "a" b "b" c "c"))) (plist-get target b))) t) (let ((target '(a "a" b "b" c "c"))) (plist-get target 'b)) ;;=3D> "b" > Also, I'd add a `cdr` to the computation of `p` so that you don't need > to do that `cdr` in both the getter and the setter. Thanks. I fix code based your advice. Finally, here is diff from last time commit. (Attached diff is squached diff. You can use this diff on last time commit if you prefer.) diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el index d52097575d..568bcf244b 100644 --- a/lisp/emacs-lisp/gv.el +++ b/lisp/emacs-lisp/gv.el @@ -421,13 +421,13 @@ plist-get (lambda (do plist prop) (macroexp-let2 macroexp-copyable-p key prop (gv-letplace (getter setter) plist - (macroexp-let2 nil p `(plist-member ,getter ,key) + (macroexp-let2 nil p `(cdr (plist-member ,getter ',key)) (funcall do `(cadr ,p) (lambda (val) - `(if (plist-member ,plist ,key) (setcar (cdr (plist-member ,plist ,key)) ,val) - ,(funcall setter `(cons ,key (cons ,val ,getter))))))))))) + `(if ,p + (setcar ,p ,val) + ,(funcall setter `(cons ',key (cons ,val ,getter))))))))))) ;;; Some occasionally handy extensions. diff --git a/test/lisp/emacs-lisp/gv-tests.el b/test/lisp/emacs-lisp/gv-tests.el index a9ceb7f730..10e3b531f3 100644 --- a/test/lisp/emacs-lisp/gv-tests.el +++ b/test/lisp/emacs-lisp/gv-tests.el @@ -182,7 +182,19 @@ gv-plist-get (should (equal (let ((target '(:a "a" :b "b" :c "c"))) (cl-rotatef (plist-get target :b) (plist-get target :d)= ) target) - '(:d "b" :a "a" :b nil :c "c")))) + '(:d "b" :a "a" :b nil :c "c"))) + + ;; Simple setf usage for plist-get. (symbol plist) + (should (equal (let ((target '(a "a" b "b" c "c"))) + (setf (plist-get target 'b) "modify") + target) + '(a "a" b "modify" c "c"))) + + ;; Other function (cl-rotatef) usage for plist-get. (symbol plist) + (should (equal (let ((target '(a "a" b "b" c "c"))) + (cl-rotatef (plist-get target 'b) (plist-get target 'c)= ) + target) + '(a "a" b "c" c "b")))) ;; `ert-deftest' messes up macroexpansion when the test file itself is ;; compiled (see Bug #24402). 2020=E5=B9=B49=E6=9C=886=E6=97=A5(=E6=97=A5) 3:03 Stefan Monnier : > Hi, > > Sorry for not answering earlier. > > > I find `gv` has `gv-define-expander` for `alist-get`, but > > there're no definition for `plist-get` > > Indeed. > > > +(gv-define-expander plist-get > > + (lambda (do plist prop) > > + (macroexp-let2 macroexp-copyable-p key prop > > + (gv-letplace (getter setter) plist > > + (macroexp-let2 nil p `(plist-member ,getter ,key) > > + (funcall do > > + `(cadr ,p) > > + (lambda (val) > > + `(if (plist-member ,plist ,key) (setcar (cdr > (plist-member ,plist ,key)) ,val) > > + ,(funcall setter `(cons ,key (cons ,val > ,getter))))))))))) > > The patch looks good (with special thanks for the tests), except that in > the code above, you shouldn't have those (repeated) `(plist-member > ,plist ,key)` since they not only waste time but they will also evaluate > `plist` multiple times which could change the semantics in case `plist` > has side effects. > I think these should refer to `p`, right? > > Furthermore, I think you need to use ',key rather than just ,key in case > the key is a cons or a plain symbol, so maybe you want to add a test > that uses symbols rather than keywords: > > ;; Other function (cl-rotatef) usage for plist-get. > (should (equal (let ((target '(a "a" b "b" c "c"))) > (cl-rotatef (plist-get target 'b) (plist-get target > 'c)) > target) > '(a "a" b "c" c "b"))) > > Also, I'd add a `cdr` to the computation of `p` so that you don't need > to do that `cdr` in both the getter and the setter. > > > -- Stefan > > --00000000000095bc8405aed70648 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
> Sorry for not answering earlier.

I thanks you for your g= reat review!

> The patch looks good (with special thanks for th= e tests), except that in
> the code above, you shouldn't have tho= se (repeated) `(plist-member
> ,plist ,key)` since they not only wast= e time but they will also evaluate
> `plist` multiple times which cou= ld change the semantics in case `plist`
> has side effects.
> I= think these should refer to `p`, right?

I think good too.

&g= t; Furthermore, I think you need to use ',key rather than just ,key in = case
> the key is a cons or a plain symbol, so maybe you want to add = a test
> that uses symbols rather than keywords:
>
> =C2= =A0 =C2=A0 ;; Other function (cl-rotatef) usage for plist-get.
> =C2= =A0 =C2=A0 (should (equal (let ((target '(a "a" b "b&quo= t; c "c")))
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(cl-rotatef (plist-get target 'b) (plist= -get target 'c))
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0target)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0'(a "a" b "c&qu= ot; c "b")))

I cannot understand this point.=C2=A0 Your ad= ditional test is passed
by current code, the additional quote is unneede= d I think.

If we add quote for `key`, the below code will be valid.<= br>
=C2=A0 =C2=A0 (let ((target '(a "a" b "b" c = "c")))
=C2=A0 =C2=A0 =C2=A0 (setf (plist-get target b) "m= odify")
=C2=A0 =C2=A0 =C2=A0 target)
=C2=A0 =C2=A0 ;;=3D> (a = "a" b "modify" c "c")

but this code wi= ll be invalid.

=C2=A0 =C2=A0 (let ((target '(a "a" b &= quot;b" c "c")))
=C2=A0 =C2=A0 =C2=A0 (setf (plist-get ta= rget 'b) "modify")
=C2=A0 =C2=A0 =C2=A0 target)
=C2=A0 = =C2=A0 ;;=3D> ('b "modify" a "a" b "b"= c "c")

I think the latter should be valid and the former = should be
invalid.=C2=A0 This is an analogy from normal usage of plist-g= et.

=C2=A0 =C2=A0 (let ((target '(a "a" b "b"= ; c "c")))
=C2=A0 =C2=A0 =C2=A0 (plist-get target b))
=C2= =A0 =C2=A0 ;;=3D> Debugger entered--Lisp error: (void-variable b)
=C2= =A0 =C2=A0 ;; =C2=A0 =C2=A0 (plist-get target b)
=C2=A0 =C2=A0 ;; =C2=A0= =C2=A0 (let ((target '(a "a" b "b" c "c"= ))) (plist-get target b))
=C2=A0 =C2=A0 ;; =C2=A0 =C2=A0 (progn (let ((t= arget '(a "a" b "b" c "c"))) (plist-get t= arget b)))
=C2=A0 =C2=A0 ;; =C2=A0 =C2=A0 eval((progn (let ((target '= ;(a "a" b "b" c "c"))) (plist-get target b)))= t)
=C2=A0 =C2=A0
=C2=A0 =C2=A0 (let ((target '(a "a" = b "b" c "c")))
=C2=A0 =C2=A0 =C2=A0 (plist-get targe= t 'b))
=C2=A0 =C2=A0 ;;=3D> "b"

> Also, I'= ;d add a `cdr` to the computation of `p` so that you don't need
>= to do that `cdr` in both the getter and the setter.

Thanks.=C2=A0 I= fix code based your advice.

Finally, here is diff from last time co= mmit.
(Attached diff is squached diff.
=C2=A0You can use this diff on= last time commit if you prefer.)

diff --git a/lisp/emacs-lisp/gv.el= b/lisp/emacs-lisp/gv.el
index d52097575d..568bcf244b 100644
--- a/li= sp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -421,13 +421,13 @@= plist-get
=C2=A0 =C2=A0(lambda (do plist prop)
=C2=A0 =C2=A0 =C2=A0(= macroexp-let2 macroexp-copyable-p key prop
=C2=A0 =C2=A0 =C2=A0 =C2=A0(g= v-letplace (getter setter) plist
- =C2=A0 =C2=A0 =C2=A0 =C2=A0(macroexp-= let2 nil p `(plist-member ,getter ,key)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0(ma= croexp-let2 nil p `(cdr (plist-member ,getter ',key))
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0(funcall do
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 `(cadr ,p)
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (lambda (val)
- =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 `(if (plist-= member ,plist ,key) (setcar (cdr (plist-member ,plist ,key)) ,val)
- =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0,(funcall setter `(cons ,key (cons ,val ,getter)))))))))))
+ =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 `(if ,p<= br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0(setcar ,p ,val)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0,(funcall setter `(cons= ',key (cons ,val ,getter)))))))))))
=C2=A0
=C2=A0;;; Some occasi= onally handy extensions.
=C2=A0
diff --git a/test/lisp/emacs-lisp/gv-= tests.el b/test/lisp/emacs-lisp/gv-tests.el
index a9ceb7f730..10e3b531f3= 100644
--- a/test/lisp/emacs-lisp/gv-tests.el
+++ b/test/lisp/emacs-= lisp/gv-tests.el
@@ -182,7 +182,19 @@ gv-plist-get
=C2=A0 =C2=A0(shou= ld (equal (let ((target '(:a "a" :b "b" :c "c&= quot;)))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 (cl-rotatef (plist-get target :b) (plist-get target :d))
=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 target)
-= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 '(:d "b&q= uot; :a "a" :b nil :c "c"))))
+ =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 '(:d "b" :a "a"= :b nil :c "c")))
+
+ =C2=A0;; Simple setf usage for plist-= get. (symbol plist)
+ =C2=A0(should (equal (let ((target '(a "a= " b "b" c "c")))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (setf (plist-get target 'b) "mo= dify")
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 target)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= '(a "a" b "modify" c "c")))
+
+ = =C2=A0;; Other function (cl-rotatef) usage for plist-get. (symbol plist)+ =C2=A0(should (equal (let ((target '(a "a" b "b"= c "c")))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 (cl-rotatef (plist-get target 'b) (plist-get target '= c))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tar= get)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 '(a &= quot;a" b "c" c "b"))))
=C2=A0
=C2=A0;; `ert= -deftest' messes up macroexpansion when the test file itself is
=C2=A0;; compiled (see Bug #24402).


2020=E5=B9= =B49=E6=9C=886=E6=97=A5(=E6=97=A5) 3:03 Stefan Monnier <monnier@iro.umontreal.ca>:
Hi,

Sorry for not answering earlier.

> I find `gv` has `gv-define-expander` for `alist-get`, but
> there're no definition for `plist-get`

Indeed.

> +(gv-define-expander plist-get
> +=C2=A0 (lambda (do plist prop)
> +=C2=A0 =C2=A0 (macroexp-let2 macroexp-copyable-p key prop
> +=C2=A0 =C2=A0 =C2=A0 (gv-letplace (getter setter) plist
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (macroexp-let2 nil p `(plist-member ,gett= er ,key)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (funcall do
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= `(cadr ,p)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= (lambda (val)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0`(if (plist-member ,plist ,key) (setcar (cdr (plist-member ,plist ,k= ey)) ,val)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 ,(funcall setter `(cons ,key (cons ,val ,getter)))))))))))
The patch looks good (with special thanks for the tests), except that in the code above, you shouldn't have those (repeated) `(plist-member
,plist ,key)` since they not only waste time but they will also evaluate `plist` multiple times which could change the semantics in case `plist`
has side effects.
I think these should refer to `p`, right?

Furthermore, I think you need to use ',key rather than just ,key in cas= e
the key is a cons or a plain symbol, so maybe you want to add a test
that uses symbols rather than keywords:

=C2=A0 =C2=A0 ;; Other function (cl-rotatef) usage for plist-get.
=C2=A0 =C2=A0 (should (equal (let ((target '(a "a" b "b&= quot; c "c")))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0(cl-rotatef (plist-get target 'b) (plist-get target 'c))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0target)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0'(= a "a" b "c" c "b")))

Also, I'd add a `cdr` to the computation of `p` so that you don't n= eed
to do that `cdr` in both the getter and the setter.


-- Stefan

--00000000000095bc8405aed70648-- --00000000000095bc8905aed7064a Content-Type: text/x-patch; charset="US-ASCII"; name="0001-Add-gv-define-expander-for-plist-get.patch" Content-Disposition: attachment; filename="0001-Add-gv-define-expander-for-plist-get.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_keuoi1uw0 RnJvbSBkODNlZTAyMjRjM2QyMzlkMjcxMzEwM2MxM2M3MWE5ZTA0OGYyMzlhIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBOYW95YSBZYW1hc2hpdGEgPGNvbmFvM0BnbWFpbC5jb20+CkRh dGU6IFdlZCwgOSBTZXAgMjAyMCAwOTo1MjozOSArMDkwMApTdWJqZWN0OiBbUEFUQ0hdIEFkZCBn di1kZWZpbmUtZXhwYW5kZXIgZm9yIHBsaXN0LWdldAoKSXQgaXMgbmVjZXNzYXJ5IHRvIG1ha2Ug cGxpc3QtZ2V0IGFzIGEgZ2VuZXJhbGl6ZWQgdmFyaWFibGUsIGFuZCB0aGlzCmRlZmluaXRpb24g YWxsb3dzIHVzZXIgdG8gdXNlIHNldGYgYW5kIG90aGVyIHVzZWZ1bCBmdW5jdGlvbnMgb24KcGxp c3QtZ2V0LgoKKiBsaXNwL2VtYWNzLWxpc3AvZ3YuZWw6IEFkZCBndi1kZWZpbmUtZXhwYW5kZXIg Zm9yIHBsaXN0LWdldAoKKiBsaXNwL2VtYWNzLWxpc3AvZ3YtdGVzdHMuZWw6IEFkZCBuZXcgdGVz dHMgZm9yIHBsaXN0LWdldAotLS0KIGxpc3AvZW1hY3MtbGlzcC9ndi5lbCAgICAgICAgICAgIHwg MTEgKysrKysrKysrCiB0ZXN0L2xpc3AvZW1hY3MtbGlzcC9ndi10ZXN0cy5lbCB8IDQwICsrKysr KysrKysrKysrKysrKysrKysrKysrKysrKysrCiAyIGZpbGVzIGNoYW5nZWQsIDUxIGluc2VydGlv bnMoKykKCmRpZmYgLS1naXQgYS9saXNwL2VtYWNzLWxpc3AvZ3YuZWwgYi9saXNwL2VtYWNzLWxp c3AvZ3YuZWwKaW5kZXggNzhkODZiOWZjMy4uNTY4YmNmMjQ0YiAxMDA2NDQKLS0tIGEvbGlzcC9l bWFjcy1saXNwL2d2LmVsCisrKyBiL2xpc3AvZW1hY3MtbGlzcC9ndi5lbApAQCAtNDE3LDYgKzQx NywxNyBAQCBhbGlzdC1nZXQKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICBgKGRlbHEgLHAgLGdldHRlcikpKSkpKQogICAgICAgICAgICAgICAgICAgICAgICAg ICAgICx2KSkpKSkpKSkpKQogCisoZ3YtZGVmaW5lLWV4cGFuZGVyIHBsaXN0LWdldAorICAobGFt YmRhIChkbyBwbGlzdCBwcm9wKQorICAgIChtYWNyb2V4cC1sZXQyIG1hY3JvZXhwLWNvcHlhYmxl LXAga2V5IHByb3AKKyAgICAgIChndi1sZXRwbGFjZSAoZ2V0dGVyIHNldHRlcikgcGxpc3QKKyAg ICAgICAgKG1hY3JvZXhwLWxldDIgbmlsIHAgYChjZHIgKHBsaXN0LW1lbWJlciAsZ2V0dGVyICcs a2V5KSkKKyAgICAgICAgICAoZnVuY2FsbCBkbworICAgICAgICAgICAgICAgICAgIGAoY2FkciAs cCkKKyAgICAgICAgICAgICAgICAgICAobGFtYmRhICh2YWwpCisgICAgICAgICAgICAgICAgICAg ICBgKGlmICxwCisgICAgICAgICAgICAgICAgICAgICAgICAgIChzZXRjYXIgLHAgLHZhbCkKKyAg ICAgICAgICAgICAgICAgICAgICAgICwoZnVuY2FsbCBzZXR0ZXIgYChjb25zICcsa2V5IChjb25z ICx2YWwgLGdldHRlcikpKSkpKSkpKSkpCiAKIDs7OyBTb21lIG9jY2FzaW9uYWxseSBoYW5keSBl eHRlbnNpb25zLgogCmRpZmYgLS1naXQgYS90ZXN0L2xpc3AvZW1hY3MtbGlzcC9ndi10ZXN0cy5l bCBiL3Rlc3QvbGlzcC9lbWFjcy1saXNwL2d2LXRlc3RzLmVsCmluZGV4IDdhODQwMmJlMDcuLjEw ZTNiNTMxZjMgMTAwNjQ0Ci0tLSBhL3Rlc3QvbGlzcC9lbWFjcy1saXNwL2d2LXRlc3RzLmVsCisr KyBiL3Rlc3QvbGlzcC9lbWFjcy1saXNwL2d2LXRlc3RzLmVsCkBAIC0xNTYsNiArMTU2LDQ2IEBA IGd2LXNldHRlci1lZGVidWcKICAgICAgIChldmFsLWJ1ZmZlcikpKQogICAoc2hvdWxkIChlcXVh bCAoZ2V0ICdndi1zZXR0ZXItZWRlYnVnICdndi1zZXR0ZXItZWRlYnVnLXByb3ApICcoMTIzKSkp KQogCisoZXJ0LWRlZnRlc3QgZ3YtcGxpc3QtZ2V0ICgpCisgIChyZXF1aXJlICdjbC1saWIpCisK KyAgOzsgU2ltcGxlIHNldGYgdXNhZ2UgZm9yIHBsaXN0LWdldC4KKyAgKHNob3VsZCAoZXF1YWwg KGxldCAoKHRhcmdldCAnKDphICJhIiA6YiAiYiIgOmMgImMiKSkpCisgICAgICAgICAgICAgICAg ICAgKHNldGYgKHBsaXN0LWdldCB0YXJnZXQgOmIpICJtb2RpZnkiKQorICAgICAgICAgICAgICAg ICAgIHRhcmdldCkKKyAgICAgICAgICAgICAgICAgJyg6YSAiYSIgOmIgIm1vZGlmeSIgOmMgImMi KSkpCisKKyAgOzsgT3RoZXIgZnVuY3Rpb24gKGNsLXJvdGF0ZWYpIHVzYWdlIGZvciBwbGlzdC1n ZXQuCisgIChzaG91bGQgKGVxdWFsIChsZXQgKCh0YXJnZXQgJyg6YSAiYSIgOmIgImIiIDpjICJj IikpKQorICAgICAgICAgICAgICAgICAgIChjbC1yb3RhdGVmIChwbGlzdC1nZXQgdGFyZ2V0IDpi KSAocGxpc3QtZ2V0IHRhcmdldCA6YykpCisgICAgICAgICAgICAgICAgICAgdGFyZ2V0KQorICAg ICAgICAgICAgICAgICAnKDphICJhIiA6YiAiYyIgOmMgImIiKSkpCisKKyAgOzsgQWRkIG5ldyBr ZXkgdmFsdWUgcGFpciBhdCB0b3Agb2YgbGlzdCBpZiBzZXRmIGZvciBtaXNzaW5nIGtleS4KKyAg KHNob3VsZCAoZXF1YWwgKGxldCAoKHRhcmdldCAnKDphICJhIiA6YiAiYiIgOmMgImMiKSkpCisg ICAgICAgICAgICAgICAgICAgKHNldGYgKHBsaXN0LWdldCB0YXJnZXQgOmQpICJtb2RpZnkiKQor ICAgICAgICAgICAgICAgICAgIHRhcmdldCkKKyAgICAgICAgICAgICAgICAgJyg6ZCAibW9kaWZ5 IiA6YSAiYSIgOmIgImIiIDpjICJjIikpKQorCisgIDs7IFJvdGF0ZSB3aXRoIG1pc3NpbmcgdmFs dWUuCisgIDs7IFRoZSB2YWx1ZSBjb3JyZXNwb25kaW5nIHRvIHRoZSBtaXNzaW5nIGtleSBpcyBh c3N1bWVkIHRvIGJlIG5pbC4KKyAgKHNob3VsZCAoZXF1YWwgKGxldCAoKHRhcmdldCAnKDphICJh IiA6YiAiYiIgOmMgImMiKSkpCisgICAgICAgICAgICAgICAgICAgKGNsLXJvdGF0ZWYgKHBsaXN0 LWdldCB0YXJnZXQgOmIpIChwbGlzdC1nZXQgdGFyZ2V0IDpkKSkKKyAgICAgICAgICAgICAgICAg ICB0YXJnZXQpCisgICAgICAgICAgICAgICAgICcoOmQgImIiIDphICJhIiA6YiBuaWwgOmMgImMi KSkpCisKKyAgOzsgU2ltcGxlIHNldGYgdXNhZ2UgZm9yIHBsaXN0LWdldC4gKHN5bWJvbCBwbGlz dCkKKyAgKHNob3VsZCAoZXF1YWwgKGxldCAoKHRhcmdldCAnKGEgImEiIGIgImIiIGMgImMiKSkp CisgICAgICAgICAgICAgICAgICAgKHNldGYgKHBsaXN0LWdldCB0YXJnZXQgJ2IpICJtb2RpZnki KQorICAgICAgICAgICAgICAgICAgIHRhcmdldCkKKyAgICAgICAgICAgICAgICAgJyhhICJhIiBi ICJtb2RpZnkiIGMgImMiKSkpCisKKyAgOzsgT3RoZXIgZnVuY3Rpb24gKGNsLXJvdGF0ZWYpIHVz YWdlIGZvciBwbGlzdC1nZXQuIChzeW1ib2wgcGxpc3QpCisgIChzaG91bGQgKGVxdWFsIChsZXQg KCh0YXJnZXQgJyhhICJhIiBiICJiIiBjICJjIikpKQorICAgICAgICAgICAgICAgICAgIChjbC1y b3RhdGVmIChwbGlzdC1nZXQgdGFyZ2V0ICdiKSAocGxpc3QtZ2V0IHRhcmdldCAnYykpCisgICAg ICAgICAgICAgICAgICAgdGFyZ2V0KQorICAgICAgICAgICAgICAgICAnKGEgImEiIGIgImMiIGMg ImIiKSkpKQorCiA7OyBgZXJ0LWRlZnRlc3QnIG1lc3NlcyB1cCBtYWNyb2V4cGFuc2lvbiB3aGVu IHRoZSB0ZXN0IGZpbGUgaXRzZWxmIGlzCiA7OyBjb21waWxlZCAoc2VlIEJ1ZyAjMjQ0MDIpLgog Ci0tIAoyLjI4LjAKCg== --00000000000095bc8905aed7064a--