Using gv in map and seq?

Using gv in map and seq?

[Nicolas Petton]

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

Hi guys,

I was wondering if it would make sense to use gv in map or seq?

I tried to make seq free of side-effects, so I'm unsure. OTOH, I think
it would be nice to have for map.el.

Nico
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Using gv in map and seq?

[Stefan Monnier]

> I was wondering if it would make sense to use gv in map or seq?

Not sure what you mean.  Do you mean to add a setter for seq-elt?


        Stefan

Re: Using gv in map and seq?

[Nicolas Petton]

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

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

>> I was wondering if it would make sense to use gv in map or seq?
>
> Not sure what you mean.  Do you mean to add a setter for seq-elt?

Yes (and a getter too?).  The same goes for `map-elt'.

Nico
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Using gv in map and seq?

[Stefan Monnier]

>>> I was wondering if it would make sense to use gv in map or seq?
>> Not sure what you mean.  Do you mean to add a setter for seq-elt?
> Yes

I see.  Yes, defining a "gv-setter" or "gv-expander" would be good.

Oh, wait, no, it's not needed, since seq-elt is an alias for `elt' and
that one already has a setter.  IOW it already works!

> (and a getter too?).

When defining a setter, you need to define the getter as well, indeed, but
that's generally the trivial part.

> The same goes for `map-elt'.

This one is not pre-defined, so yes, we should define it.
Actually, this just means to turn map-put into a gv-setter.

Something like the patch below seems to work (and it generalizes
map-put to accept for MAP not only symbols but any "lvalue").  It also
tightens the code generated by map--dispatch.


        Stefan


diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el
index dd7fb91..26ad6ce 100644
--- a/lisp/emacs-lisp/map.el
+++ b/lisp/emacs-lisp/map.el
@@ -82,25 +82,19 @@ The following keyword types are meaningful: `:list',
 
 An error is thrown if MAP is neither a list, hash-table nor array.
 
-Return RESULT if non-nil or the result of evaluation of the
-form.
+Return RESULT if non-nil or the result of evaluation of the form.
 
 \(fn (VAR MAP [RESULT]) &rest ARGS)"
   (declare (debug t) (indent 1))
   (unless (listp spec)
     (setq spec `(,spec ,spec)))
-  (let ((map-var (car spec))
-        (result-var (make-symbol "result")))
-    `(let ((,map-var ,(cadr spec))
-           ,result-var)
-       (setq ,result-var
-             (cond ((listp ,map-var) ,(plist-get args :list))
-                   ((hash-table-p ,map-var) ,(plist-get args :hash-table))
-                   ((arrayp ,map-var) ,(plist-get args :array))
-                   (t (error "Unsupported map: %s" ,map-var))))
-       ,@(when (cddr spec)
-           `((setq ,result-var ,@(cddr spec))))
-       ,result-var)))
+  (let ((map-var (car spec)))
+    `(let* ,(unless (eq map-var (cadr spec)) `((,map-var ,(cadr spec))))
+       (cond ((listp ,map-var) ,(plist-get args :list))
+             ((hash-table-p ,map-var) ,(plist-get args :hash-table))
+             ((arrayp ,map-var) ,(plist-get args :array))
+             (t (error "Unsupported map: %s" ,map-var)))
+       ,@(cddr spec))))
 
 (defun map-elt (map key &optional default)
   "Perform a lookup in MAP of KEY and return its associated value.
@@ -109,27 +103,23 @@ If KEY is not found, return DEFAULT which defaults to nil.
 If MAP is a list, `equal' is used to lookup KEY.
 
 MAP can be a list, hash-table or array."
+  (declare
+   (gv-expander
+    (lambda (do)
+      (macroexp-let2* nil
+          ;; Eval them once and for all in the right order.
+          ((map map) (key key) (default default))
+        `(map--dispatch map
+           :list ,(gv-get `(alist-get ,key ,map ,default) do)
+           :hash-table ,(funcall do `(gethash ,key ,map ,default)
+                                 (lambda (v) `(puthash ,key ,v ,map)))
+           :array ,(funcall do `(aref ,map ,key)
+                            (lambda (v) `(aset ,map ,key ,v))))))))
   (map--dispatch map
     :list (map--elt-list map key default)
     :hash-table (gethash key map default)
     :array (map--elt-array map key default)))
 
-(defmacro map-put (map key value)
-  "In MAP, associate KEY with VALUE and return MAP.
-If KEY is already present in MAP, replace the associated value
-with VALUE.
-
-MAP can be a list, hash-table or array."
-  (declare (debug t))
-  (let ((symbol (symbolp map)))
-    `(progn
-       (map--dispatch (m ,map m)
-         :list (if ,symbol
-                   (setq ,map (cons (cons ,key ,value) m))
-                 (error "Literal lists are not allowed, %s must be a symbol" ',map))
-         :hash-table (puthash ,key ,value m)
-         :array (aset m ,key ,value)))))
-
 (defmacro map-delete (map key)
   "In MAP, delete the key KEY if present and return MAP.
 If MAP is an array, store nil at the index KEY.

Re: Using gv in map and seq?

[Stefan Monnier]

> Something like the patch below seems to work (and it generalizes
> map-put to accept for MAP not only symbols but any "lvalue").  It also
> tightens the code generated by map--dispatch.

BTW, just like `map-put', it suffers from the major problem that it
hardcodes in the macroexpanded code (i.e. in the .elc files of Elisp
packages that use map-put) the current definition of map--dispatch and
the current 3 supported map types.

So we should introduce a helper function (call it map--put) and move
most of the setter's code there (including the dispatch).


        Stefan

Re: Using gv in map and seq?

[Nicolas Petton]

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

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

>> Something like the patch below seems to work (and it generalizes
>> map-put to accept for MAP not only symbols but any "lvalue").  It also
>> tightens the code generated by map--dispatch.
>
> BTW, just like `map-put', it suffers from the major problem that it
> hardcodes in the macroexpanded code (i.e. in the .elc files of Elisp
> packages that use map-put) the current definition of map--dispatch and
> the current 3 supported map types.

Indeed, I never though about this issue!

> So we should introduce a helper function (call it map--put) and move
> most of the setter's code there (including the dispatch).

Yes, I can do that.
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Using gv in map and seq?

[Nicolas Petton]

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

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

>> Something like the patch below seems to work (and it generalizes
>> map-put to accept for MAP not only symbols but any "lvalue").  It also
>> tightens the code generated by map--dispatch.

But you have completely removed `map-put' here, right?  Is it supposed to
be implemented in terms of `map-elt' now that it is "set-able"?

Nico
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Using gv in map and seq?

[Nicolas Petton]

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

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

> Something like the patch below seems to work (and it generalizes
> map-put to accept for MAP not only symbols but any "lvalue").

For lists, yes indeed. I was looking for a simple way to do that,
thanks!

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Using gv in map and seq?

[Stefan Monnier]

>>> Something like the patch below seems to work (and it generalizes
>>> map-put to accept for MAP not only symbols but any "lvalue").  It also
>>> tightens the code generated by map--dispatch.
> But you have completely removed `map-put' here, right?  Is it supposed to
> be implemented in terms of `map-elt' now that it is "set-able"?

Indeed, it becomes redundant: you can use (setf (map-elt M K) V) à la place.

> > Something like the patch below seems to work (and it generalizes
> > map-put to accept for MAP not only symbols but any "lvalue").
> For lists, yes indeed. I was looking for a simple way to do that,
> thanks!

Since the macro-expansion is done before we know what type we'll get,
the restriction to lvalues carries over to all other types :-(

> Je bloque sur ton code, quand j'évalue
>
>     (setq my-map '())
>     (setf (map-elt my-map 'b) 4)
>
> J'ai une erreur `Debugger entered--Lisp error: (void-variable map)'

Il y a en tout cas l'erreur ci-dessous qu'il faut corriger:

> > +        `(map--dispatch map
                             ^^^
                             ,map

-- Stefan

Re: Using gv in map and seq?

[Stefan Monnier]

>> > Something like the patch below seems to work (and it generalizes
>> > map-put to accept for MAP not only symbols but any "lvalue").
>> For lists, yes indeed. I was looking for a simple way to do that,
>> thanks!
> Since the macro-expansion is done before we know what type we'll get,
> the restriction to lvalues carries over to all other types :-(

Hmm.. actually I guess we could try to catch the macro-expansion error
in the list-branch (when the MAP expression is not an lvalue) and
still generate the code for the other branches.


        Stefan

Re: Using gv in map and seq?

[Nicolas Petton]

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

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

>>> > Something like the patch below seems to work (and it generalizes
>>> > map-put to accept for MAP not only symbols but any "lvalue").
>>> For lists, yes indeed. I was looking for a simple way to do that,
>>> thanks!
>> Since the macro-expansion is done before we know what type we'll get,
>> the restriction to lvalues carries over to all other types :-(

Just out of curiosity, where is this restriction set?

Nico
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Using gv in map and seq?

[Stefan Monnier]

> Just out of curiosity, where is this restriction set?

The code I sent was broken (in the list case where we need to set the
MAP, the code does (setq map ..) where `map' is an internal variable,
IOW it sets the wrong thing) so the restriction didn't actually appear.

It should look a bit more like:

  (put 'map--raw-place 'gv-expander #'funcall)
  [...]
    (declare
     (gv-expander
      (lambda (do)
        (gv-letplace (mgetter msetter) map
          (macroexp-let2* nil
              ;; Eval them once and for all in the right order.
              ((key key) (default default))
            `(map--dispatch ,mgetter
               :list ,(gv-get `(alist-get ,key (map--raw-place ,mgetter ,msetter)
                                          ,default)
                              do)
               :hash-table ,(funcall do `(gethash ,key ,mgetter ,default)
                                     (lambda (v) `(puthash ,key ,v ,mgetter)))
               :array ,(funcall do `(aref ,mgetter ,key)
                                (lambda (v) `(aset ,mgetter ,key ,v)))))))))

And this "raw-place" should probably be added to gv.el.


        Stefan

[OT] Working with patches inlined in emails (was: Using gv in map and seq?)

[Nicolas Petton]

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

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

> diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el
> index dd7fb91..26ad6ce 100644
> --- a/lisp/emacs-lisp/map.el
> +++ b/lisp/emacs-lisp/map.el
> @@ -82,25 +82,19 @@ The following keyword types are meaningful: `:list',
>  
>  An error is thrown if MAP is neither a list, hash-table nor array.
>  
> -Return RESULT if non-nil or the result of evaluation of the
> -form.
> +Return RESULT if non-nil or the result of evaluation of the form.
>  
>  \(fn (VAR MAP [RESULT]) &rest ARGS)"
>    (declare (debug t) (indent 1))
>    (unless (listp spec)
>      (setq spec `(,spec ,spec)))
> -  (let ((map-var (car spec))
> -        (result-var (make-symbol "result")))
> -    `(let ((,map-var ,(cadr spec))
> -           ,result-var)
> -       (setq ,result-var
> -             (cond ((listp ,map-var) ,(plist-get args :list))
> -                   ((hash-table-p ,map-var) ,(plist-get args :hash-table))
> -                   ((arrayp ,map-var) ,(plist-get args :array))
> -                   (t (error "Unsupported map: %s" ,map-var))))
> -       ,@(when (cddr spec)
> -           `((setq ,result-var ,@(cddr spec))))
> -       ,result-var)))
> +  (let ((map-var (car spec)))
> +    `(let* ,(unless (eq map-var (cadr spec)) `((,map-var ,(cadr spec))))
> +       (cond ((listp ,map-var) ,(plist-get args :list))
> +             ((hash-table-p ,map-var) ,(plist-get args :hash-table))
> +             ((arrayp ,map-var) ,(plist-get args :array))
> +             (t (error "Unsupported map: %s" ,map-var)))
> +       ,@(cddr spec))))
>  
>  (defun map-elt (map key &optional default)
>    "Perform a lookup in MAP of KEY and return its associated value.
> @@ -109,27 +103,23 @@ If KEY is not found, return DEFAULT which defaults to nil.
>  If MAP is a list, `equal' is used to lookup KEY.
>  
>  MAP can be a list, hash-table or array."
> +  (declare
> +   (gv-expander
> +    (lambda (do)
> +      (macroexp-let2* nil
> +          ;; Eval them once and for all in the right order.
> +          ((map map) (key key) (default default))
> +        `(map--dispatch map
> +           :list ,(gv-get `(alist-get ,key ,map ,default) do)
> +           :hash-table ,(funcall do `(gethash ,key ,map ,default)
> +                                 (lambda (v) `(puthash ,key ,v ,map)))
> +           :array ,(funcall do `(aref ,map ,key)
> +                            (lambda (v) `(aset ,map ,key ,v))))))))
>    (map--dispatch map
>      :list (map--elt-list map key default)
>      :hash-table (gethash key map default)
>      :array (map--elt-array map key default)))
>  
> -(defmacro map-put (map key value)
> -  "In MAP, associate KEY with VALUE and return MAP.
> -If KEY is already present in MAP, replace the associated value
> -with VALUE.
> -
> -MAP can be a list, hash-table or array."
> -  (declare (debug t))
> -  (let ((symbol (symbolp map)))
> -    `(progn
> -       (map--dispatch (m ,map m)
> -         :list (if ,symbol
> -                   (setq ,map (cons (cons ,key ,value) m))
> -                 (error "Literal lists are not allowed, %s must be a symbol" ',map))
> -         :hash-table (puthash ,key ,value m)
> -         :array (aset m ,key ,value)))))
> -
>  (defmacro map-delete (map key)
>    "In MAP, delete the key KEY if present and return MAP.
>  If MAP is an array, store nil at the index KEY.


This is a bit off-topic, but I'm wondering how people work with emails
in Emacs containing patches in the body of the email.

With patches sent as attachments it's straightforward, but for example
here for this patch I had to yank the content in a new buffer, trim the
spaces at the beginning of each line, etc. which is clearly not
efficient (I use notmuch).

Cheers,
Nico
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [OT] Working with patches inlined in emails (was: Using gv in map and seq?)

[Eli Zaretskii]

> From: Nicolas Petton <nicolas@petton.fr>
> Date: Thu, 18 Jun 2015 10:31:20 +0200
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> This is a bit off-topic, but I'm wondering how people work with emails
> in Emacs containing patches in the body of the email.

With "M-|", of course.

> With patches sent as attachments it's straightforward, but for example
> here for this patch I had to yank the content in a new buffer, trim the
> spaces at the beginning of each line, etc. which is clearly not
> efficient (I use notmuch).

I don't understand why you needed to trim some spaces, I don't see any
extra spaces in the mail Stefan sent.  Maybe your MUA needs to be
upgraded or changed?

Anyway, 'patch' has a switch to ignore whitespace, which helps in some
situations.

Re: [OT] Working with patches inlined in emails

[Andreas Schwab]

Nicolas Petton <nicolas@petton.fr> writes:

> This is a bit off-topic, but I'm wondering how people work with emails
> in Emacs containing patches in the body of the email.

Just pipe it to patch.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [OT] Working with patches inlined in emails

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Nicolas Petton <nicolas@petton.fr>
>> Date: Thu, 18 Jun 2015 10:31:20 +0200
>> Cc: emacs-devel <emacs-devel@gnu.org>
>> 
>> This is a bit off-topic, but I'm wondering how people work with emails
>> in Emacs containing patches in the body of the email.
>
> With "M-|", of course.

Here's an alternative that might a bit more portable, emacs-centric and
user-friendly: just yank the patch region to a new buffer and `M-x
diff-mode' there. Then you can `C-c C-c` and `C-c C-a` at will,
applying, undoing and reading the differences as you go.

If you're lazy, don't mark and yank to a new buffer, just `M-x
diff-mode` whatever buffer you find the message in.

diff-mode will ask you to locate the file. Alternatively, use `M-x cd`
on that buffer to switch to the root of the project the patch pertains
too. I think I've seen diff-mode automagically guessing the file but I'm
too lazy right now to figure out and describe under which conditions
that happens, if at all.

João

Re: [OT] Working with patches inlined in emails (was: Using gv in map and seq?)

[Nicolas Petton]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Nicolas Petton <nicolas@petton.fr>
>> Date: Thu, 18 Jun 2015 10:31:20 +0200
>> Cc: emacs-devel <emacs-devel@gnu.org>
>> 
>> This is a bit off-topic, but I'm wondering how people work with emails
>> in Emacs containing patches in the body of the email.
>
> With "M-|", of course.
>
>> With patches sent as attachments it's straightforward, but for example
>> here for this patch I had to yank the content in a new buffer, trim the
>> spaces at the beginning of each line, etc. which is clearly not
>> efficient (I use notmuch).
>
> I don't understand why you needed to trim some spaces, I don't see any
> extra spaces in the mail Stefan sent.  Maybe your MUA needs to be
> upgraded or changed?

I couldn't just pipe it to patch as there were some spaces at the
beginning of each line, but that was indeed specific to notmuch
indenting messages in threads (and it's customizable, so I just disabled
it).

Sorry for the noise.
Nico
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [OT] Working with patches inlined in emails (was: Using gv in map and seq?)

[Nicolas Petton]

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

Nicolas Petton <nicolas@petton.fr> writes:

> I couldn't just pipe it to patch as there were some spaces at the
> beginning of each line, but that was indeed specific to notmuch
> indenting messages in threads (and it's customizable, so I just disabled
> it).

Just in case somebody's interested in notmuch and inline patches, I just
found out that notmuch can convert inline patches into 'text/x-diff'
attachments with `notmuch-wash-convert-inline-patch-to-part'.

Nico
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Using gv in map and seq?

[Nicolas Petton]

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

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Just out of curiosity, where is this restriction set?
>
> The code I sent was broken (in the list case where we need to set the
> MAP, the code does (setq map ..) where `map' is an internal variable,
> IOW it sets the wrong thing) so the restriction didn't actually appear.
>
> It should look a bit more like:
>
>   (put 'map--raw-place 'gv-expander #'funcall)
>   [...]
>     (declare
>      (gv-expander
>       (lambda (do)
>         (gv-letplace (mgetter msetter) map
>           (macroexp-let2* nil
>               ;; Eval them once and for all in the right order.
>               ((key key) (default default))
>             `(map--dispatch ,mgetter
>                :list ,(gv-get `(alist-get ,key (map--raw-place ,mgetter ,msetter)
>                                           ,default)
>                               do)
>                :hash-table ,(funcall do `(gethash ,key ,mgetter ,default)
>                                      (lambda (v) `(puthash ,key ,v ,mgetter)))
>                :array ,(funcall do `(aref ,mgetter ,key)
>                                 (lambda (v) `(aset ,mgetter ,key ,v)))))))))
>
> And this "raw-place" should probably be added to gv.el.

You lost me :-)
Could you explain a bit more?

Nico
-- 
Nicolas Petton
http://nicolas-petton.fr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: Using gv in map and seq?

[Stefan Monnier]

> You lost me :-)
> Could you explain a bit more?

Quelle partie?

gv-letplace prend une "place" est la transforme en un "getter" et un
"setter".  P.ex. si `map' est (nth 5 (foo)), gv-letplace va ajouter un
"(let ((x (nthcdr 5 (foo)))) ...)" et renvoyer la sexp "(car x)" comme
"getter" et la fonction (lambda (v) `(setcar x ,v)) comme "setter".

Ça doit être au tout debut du gv-expander, vu que l'évaluation de
(nthcdr 5 (foo)) doit avoir lieu une seule fois et avant l'évaluation de
`key', `default', etc...

Ensuite, on veut passer cette "place" à `alist-get' (pour éviter de
réinventer son fonctionnement), mais on peut plus utiliser `map' (qui
causerait une deuxième évaluation de (nthcdr 5 (foo)), et donc une
erreur) donc il faut synthétiser une place à partir de "mgetter" et
"msetter", et c'èst à ça que sert le `map--raw-place'.


        Stefan