all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] Add gv-define-expander for plist-get
@ 2020-08-17  9:05 Naoya Yamashita
  2020-08-27  4:35 ` Naoya Yamashita
  2020-09-05 18:03 ` Stefan Monnier
  0 siblings, 2 replies; 9+ messages in thread
From: Naoya Yamashita @ 2020-08-17  9:05 UTC (permalink / raw)
  To: emacs-devel@gnu.org


[-- Attachment #1.1: Type: text/plain, Size: 955 bytes --]

Hello, all.

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

Therefore, `setf` works only for `alist-get` but not for
`plist-get`.

```
(let ((target '((a . "a") (b . "b"))))
  (setf (alist-get 'a target) "modify")
  target)
;;=> ((a . "modify") (b . "b"))

(let ((target '(:a "a" :b "b")))
  (setf (plist-get target :a) "modify")
  target)
;;=> Debugger entered--Lisp error: (void-function \(setf\ plist-get\))
;;     (\(setf\ plist-get\) "modify" v :a)
;;     (let* ((v v)) (\(setf\ plist-get\) "modify" v :a))
;;     (setf (plist-get v :a) "modify")
;;     (let ((v '(:a "a" :b "b"))) (setf (plist-get v :a) "modify"))
;;     (progn (let ((v '(:a "a" :b "b"))) (setf (plist-get v :a) "modify")))
;;     eval((progn (let ((v '(:a "a" :b "b"))) (setf (plist-get v :a)
"modify"))) t)
```

This patch adds the definition of gv-expander for `plist-get`.
Please refer to the additional test cases for usage.

[-- Attachment #1.2: Type: text/html, Size: 1407 bytes --]

[-- Attachment #2: 0001-Add-gv-define-expander-for-plist-get.patch --]
[-- Type: text/x-patch, Size: 3038 bytes --]

From e43c884a60fdffbedc6a94d86ddbe24ea6918220 Mon Sep 17 00:00:00 2001
From: conao3 <conao3@gmail.com>
Date: Mon, 17 Aug 2020 16:56:37 +0900
Subject: [PATCH] Add gv-define-expander for plist-get

It is necessary to make plist-get as a generalized variable, and this
definition allows user to use setf and other useful functions on
plist-get.

* lisp/emacs-lisp/gv.el: Add gv-define-expander for plist-get

* lisp/emacs-lisp/gv-tests.el: Add new test for plist-get
---
 lisp/emacs-lisp/gv.el            | 10 ++++++++++
 test/lisp/emacs-lisp/gv-tests.el | 28 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index 513bd32889..604c62c301 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -417,6 +417,16 @@ alist-get
                                               `(delq ,p ,getter))))))
                             ,v))))))))))
 
+(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)))))))))))
 
 ;;; Some occasionally handy extensions.
 
diff --git a/test/lisp/emacs-lisp/gv-tests.el b/test/lisp/emacs-lisp/gv-tests.el
index 7a8402be07..a9ceb7f730 100644
--- a/test/lisp/emacs-lisp/gv-tests.el
+++ b/test/lisp/emacs-lisp/gv-tests.el
@@ -156,6 +156,34 @@ gv-setter-edebug
       (eval-buffer)))
   (should (equal (get 'gv-setter-edebug 'gv-setter-edebug-prop) '(123))))
 
+(ert-deftest gv-plist-get ()
+  (require 'cl-lib)
+
+  ;; Simple setf usage for plist-get.
+  (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.
+  (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")))
+
+  ;; Add new key value pair at top of list if setf for missing key.
+  (should (equal (let ((target '(:a "a" :b "b" :c "c")))
+                   (setf (plist-get target :d) "modify")
+                   target)
+                 '(:d "modify" :a "a" :b "b" :c "c")))
+
+  ;; Rotate with missing value.
+  ;; The value corresponding to the missing key is assumed to be nil.
+  (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"))))
+
 ;; `ert-deftest' messes up macroexpansion when the test file itself is
 ;; compiled (see Bug #24402).
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add gv-define-expander for plist-get
  2020-08-17  9:05 [PATCH] Add gv-define-expander for plist-get Naoya Yamashita
@ 2020-08-27  4:35 ` Naoya Yamashita
  2020-09-05 18:03 ` Stefan Monnier
  1 sibling, 0 replies; 9+ messages in thread
From: Naoya Yamashita @ 2020-08-27  4:35 UTC (permalink / raw)
  To: emacs-devel@gnu.org

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

Does anyone see this email?
Let me know if there's a better place to put this patch out.
Excuse me, I'm not used to communicating on ML...

2020年8月17日(月) 18:05 Naoya Yamashita <conao3@gmail.com>:

> Hello, all.
>
> I find `gv` has `gv-define-expander` for `alist-get`, but
> there're no definition for `plist-get`
>
> Therefore, `setf` works only for `alist-get` but not for
> `plist-get`.
>
> ```
> (let ((target '((a . "a") (b . "b"))))
>   (setf (alist-get 'a target) "modify")
>   target)
> ;;=> ((a . "modify") (b . "b"))
>
> (let ((target '(:a "a" :b "b")))
>   (setf (plist-get target :a) "modify")
>   target)
> ;;=> Debugger entered--Lisp error: (void-function \(setf\ plist-get\))
> ;;     (\(setf\ plist-get\) "modify" v :a)
> ;;     (let* ((v v)) (\(setf\ plist-get\) "modify" v :a))
> ;;     (setf (plist-get v :a) "modify")
> ;;     (let ((v '(:a "a" :b "b"))) (setf (plist-get v :a) "modify"))
> ;;     (progn (let ((v '(:a "a" :b "b"))) (setf (plist-get v :a)
> "modify")))
> ;;     eval((progn (let ((v '(:a "a" :b "b"))) (setf (plist-get v :a)
> "modify"))) t)
> ```
>
> This patch adds the definition of gv-expander for `plist-get`.
> Please refer to the additional test cases for usage.
>
>
>

[-- Attachment #2: Type: text/html, Size: 1927 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add gv-define-expander for plist-get
  2020-08-17  9:05 [PATCH] Add gv-define-expander for plist-get Naoya Yamashita
  2020-08-27  4:35 ` Naoya Yamashita
@ 2020-09-05 18:03 ` Stefan Monnier
  2020-09-09  1:03   ` Naoya Yamashita
  1 sibling, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2020-09-05 18:03 UTC (permalink / raw)
  To: Naoya Yamashita; +Cc: emacs-devel@gnu.org

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




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add gv-define-expander for plist-get
  2020-09-05 18:03 ` Stefan Monnier
@ 2020-09-09  1:03   ` Naoya Yamashita
  2020-09-09  3:23     ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Naoya Yamashita @ 2020-09-09  1:03 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel@gnu.org


[-- Attachment #1.1: Type: text/plain, Size: 6224 bytes --]

> 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)
    ;;=> (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)
    ;;=> ('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))
    ;;=> 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))
    ;;=> "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年9月6日(日) 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
> > +  (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
>
>

[-- Attachment #1.2: Type: text/html, Size: 8198 bytes --]

[-- Attachment #2: 0001-Add-gv-define-expander-for-plist-get.patch --]
[-- Type: text/x-patch, Size: 3547 bytes --]

From d83ee0224c3d239d2713103c13c71a9e048f239a Mon Sep 17 00:00:00 2001
From: Naoya Yamashita <conao3@gmail.com>
Date: Wed, 9 Sep 2020 09:52:39 +0900
Subject: [PATCH] Add gv-define-expander for plist-get

It is necessary to make plist-get as a generalized variable, and this
definition allows user to use setf and other useful functions on
plist-get.

* lisp/emacs-lisp/gv.el: Add gv-define-expander for plist-get

* lisp/emacs-lisp/gv-tests.el: Add new tests for plist-get
---
 lisp/emacs-lisp/gv.el            | 11 +++++++++
 test/lisp/emacs-lisp/gv-tests.el | 40 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index 78d86b9fc3..568bcf244b 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -417,6 +417,17 @@ alist-get
                                               `(delq ,p ,getter))))))
                             ,v))))))))))
 
+(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 `(cdr (plist-member ,getter ',key))
+          (funcall do
+                   `(cadr ,p)
+                   (lambda (val)
+                     `(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 7a8402be07..10e3b531f3 100644
--- a/test/lisp/emacs-lisp/gv-tests.el
+++ b/test/lisp/emacs-lisp/gv-tests.el
@@ -156,6 +156,46 @@ gv-setter-edebug
       (eval-buffer)))
   (should (equal (get 'gv-setter-edebug 'gv-setter-edebug-prop) '(123))))
 
+(ert-deftest gv-plist-get ()
+  (require 'cl-lib)
+
+  ;; Simple setf usage for plist-get.
+  (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.
+  (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")))
+
+  ;; Add new key value pair at top of list if setf for missing key.
+  (should (equal (let ((target '(:a "a" :b "b" :c "c")))
+                   (setf (plist-get target :d) "modify")
+                   target)
+                 '(:d "modify" :a "a" :b "b" :c "c")))
+
+  ;; Rotate with missing value.
+  ;; The value corresponding to the missing key is assumed to be nil.
+  (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")))
+
+  ;; 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).
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add gv-define-expander for plist-get
  2020-09-09  1:03   ` Naoya Yamashita
@ 2020-09-09  3:23     ` Stefan Monnier
  2020-09-09 12:32       ` Naoya Yamashita
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2020-09-09  3:23 UTC (permalink / raw)
  To: Naoya Yamashita; +Cc: emacs-devel@gnu.org

> I cannot understand this point.  Your additional test is passed
> by current code, the additional quote is unneeded I think.

Looks like I was just confused, thanks for double checking.

>    (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)
                        ^^^^
                        car?

Could you send me the patch against `master`, so I can install it?


        Stefan




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add gv-define-expander for plist-get
  2020-09-09  3:23     ` Stefan Monnier
@ 2020-09-09 12:32       ` Naoya Yamashita
  2020-09-09 17:51         ` Stefan Monnier
  0 siblings, 1 reply; 9+ messages in thread
From: Naoya Yamashita @ 2020-09-09 12:32 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel@gnu.org


[-- Attachment #1.1: Type: text/plain, Size: 1213 bytes --]

> Looks like I was just confused, thanks for double checking.

OK, your welcome (and my patch is mistaken same issue)

>>                     `(cadr ,p)
>                        ^^^^
>                        car?

Thanks. I change this code repeatedly and squash many time,
I was confused at that time.

> Could you send me the patch against `master`, so I can install it?

OK, I recreated the patch and attached it.




2020年9月9日(水) 12:23 Stefan Monnier <monnier@iro.umontreal.ca>:

> > I cannot understand this point.  Your additional test is passed
> > by current code, the additional quote is unneeded I think.
>
> Looks like I was just confused, thanks for double checking.
>
> >    (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)
>                         ^^^^
>                         car?
>
> Could you send me the patch against `master`, so I can install it?
>
>
>         Stefan
>
>

[-- Attachment #1.2: Type: text/html, Size: 1954 bytes --]

[-- Attachment #2: 0001-Add-gv-define-expander-for-plist-get.patch --]
[-- Type: text/x-patch, Size: 3544 bytes --]

From 3494fa7108c9df8d2f5b93c6d978b88943f91780 Mon Sep 17 00:00:00 2001
From: Naoya Yamashita <conao3@gmail.com>
Date: Wed, 9 Sep 2020 09:52:39 +0900
Subject: [PATCH] Add gv-define-expander for plist-get

It is necessary to make plist-get as a generalized variable, and this
definition allows user to use setf and other useful functions on
plist-get.

* lisp/emacs-lisp/gv.el: Add gv-define-expander for plist-get

* lisp/emacs-lisp/gv-tests.el: Add new tests for plist-get
---
 lisp/emacs-lisp/gv.el            | 11 +++++++++
 test/lisp/emacs-lisp/gv-tests.el | 40 ++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index 78d86b9fc3..5470b8532f 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -417,6 +417,17 @@ alist-get
                                               `(delq ,p ,getter))))))
                             ,v))))))))))
 
+(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 `(cdr (plist-member ,getter ,key))
+          (funcall do
+                   `(car ,p)
+                   (lambda (val)
+                     `(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 7a8402be07..10e3b531f3 100644
--- a/test/lisp/emacs-lisp/gv-tests.el
+++ b/test/lisp/emacs-lisp/gv-tests.el
@@ -156,6 +156,46 @@ gv-setter-edebug
       (eval-buffer)))
   (should (equal (get 'gv-setter-edebug 'gv-setter-edebug-prop) '(123))))
 
+(ert-deftest gv-plist-get ()
+  (require 'cl-lib)
+
+  ;; Simple setf usage for plist-get.
+  (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.
+  (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")))
+
+  ;; Add new key value pair at top of list if setf for missing key.
+  (should (equal (let ((target '(:a "a" :b "b" :c "c")))
+                   (setf (plist-get target :d) "modify")
+                   target)
+                 '(:d "modify" :a "a" :b "b" :c "c")))
+
+  ;; Rotate with missing value.
+  ;; The value corresponding to the missing key is assumed to be nil.
+  (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")))
+
+  ;; 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).
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add gv-define-expander for plist-get
  2020-09-09 12:32       ` Naoya Yamashita
@ 2020-09-09 17:51         ` Stefan Monnier
  2020-09-09 20:08           ` Naoya Yamashita
  2020-09-10 12:40           ` Adam Porter
  0 siblings, 2 replies; 9+ messages in thread
From: Stefan Monnier @ 2020-09-09 17:51 UTC (permalink / raw)
  To: Naoya Yamashita; +Cc: emacs-devel@gnu.org

> OK, I recreated the patch and attached it.

Thanks, installed,


        Stefan




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add gv-define-expander for plist-get
  2020-09-09 17:51         ` Stefan Monnier
@ 2020-09-09 20:08           ` Naoya Yamashita
  2020-09-10 12:40           ` Adam Porter
  1 sibling, 0 replies; 9+ messages in thread
From: Naoya Yamashita @ 2020-09-09 20:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel@gnu.org

[-- Attachment #1: Type: text/plain, Size: 187 bytes --]

Thanks!

2020年9月10日(木) 2:51 Stefan Monnier <monnier@iro.umontreal.ca>:

> > OK, I recreated the patch and attached it.
>
> Thanks, installed,
>
>
>         Stefan
>
>

[-- Attachment #2: Type: text/html, Size: 489 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add gv-define-expander for plist-get
  2020-09-09 17:51         ` Stefan Monnier
  2020-09-09 20:08           ` Naoya Yamashita
@ 2020-09-10 12:40           ` Adam Porter
  1 sibling, 0 replies; 9+ messages in thread
From: Adam Porter @ 2020-09-10 12:40 UTC (permalink / raw)
  To: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> OK, I recreated the patch and attached it.
>
> Thanks, installed,

This is a very welcome addition to Elisp.  Thanks to Naoya for putting
it together.

I think this deserves to be mentioned in the NEWS file, otherwise
Elispers may not realize this new tool is available.  :)




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-09-10 12:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-17  9:05 [PATCH] Add gv-define-expander for plist-get Naoya Yamashita
2020-08-27  4:35 ` Naoya Yamashita
2020-09-05 18:03 ` Stefan Monnier
2020-09-09  1:03   ` Naoya Yamashita
2020-09-09  3:23     ` Stefan Monnier
2020-09-09 12:32       ` Naoya Yamashita
2020-09-09 17:51         ` Stefan Monnier
2020-09-09 20:08           ` Naoya Yamashita
2020-09-10 12:40           ` Adam Porter

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.