magnars / dash.el

A modern list library for Emacs
GNU General Public License v3.0
1.67k stars 137 forks source link

-defun macro to flexibly handle arguments (better than cl-defun) #268

Open alphapapa opened 6 years ago

alphapapa commented 6 years ago

Wanting more flexible argument handling, I tried to implement this with pcase, but I couldn't get it to match the plist at the end. It was much easier with -let, however I ran into a problem that I'm not sure how to overcome, or if it's even possible to do so.

(defmacro -defun (name arg-patterns &rest body)
  (declare (indent defun))
  (-let ((body-sym (gensym))
         ((arg-patterns defaults) (cl-loop with patterns with defaults
                                           for pattern = (pop arg-patterns)
                                           until (eq pattern :defaults)
                                           collect pattern into patterns
                                           finally return (list patterns (car arg-patterns)))))
    `(defun ,name (&rest args)
       (let ((,body-sym (lambda ()
                          ,@body))
             ,@defaults)
         (or ,@(--map `(-when-let (,it args) (funcall ,body-sym))
                      arg-patterns)
             (user-error ,(concat (symbol-name name) ": Called with invalid args: %s") args))))))

(-defun nuts ((type number &keys :salted salted)
              (type &keys :salted salted)
              (type number)
              (type)
              :defaults ((number 10) (salted t)))
  (list :type type :number number :salted salted))

(nuts 'peanuts)  ;; => (:type peanuts :number 10 :salted t)
(nuts 'peanuts 20)  ;; => (:type peanuts :number 20 :salted t)
(nuts 'peanuts 20 :salted t)  ;; => (:type peanuts :number 20 :salted t)

That all works great, but when I try to set :salted to nil, it doesn't work, because -let doesn't match the plist key's nil value:

(nuts 'peanuts :salted nil)  ;; => (:type peanuts :number 10 :salted t)

If this could be fixed, this could be a really useful way to define functions that accept arguments. It would allow one to avoid manually looping over argument lists.

Any ideas would be appreciated. :)

In case anyone is interested, here's my aborted attempt using pcase:

(defmacro pcase-lambda* (arg-patterns &rest body)
  (declare (indent defun))
  (let ((body-sym (gensym)))
    `(lambda (&rest args)
       (let ((,body-sym (lambda ()
                          ,@body)))
         (pcase args
           ,@(--map `(,it (funcall ,body-sym))
                    arg-patterns)
           (_ (user-error "Called with invalid args")))))))

(setq argh (pcase-lambda* (`(,files ,pred . ,(map something)))
             ;; TODO: default values
             (list :files files :pred pred :something something)))

(funcall argh 'files 'pred 'something 'something)  ;; => (:files files :pred pred :something nil)
Fuco1 commented 6 years ago

-let can destructure nil fine

(-let (((&plist :foo foo) (list :foo nil)))
  foo)

but you are using -when-let which fails if any binding is nil, that's probably the issue.

(-if-let (((&plist :foo foo) (list :foo nil)))
    "yes"
  "no")
Fuco1 commented 6 years ago

Is there any reason why you have multiple signatures

((type number &keys :salted salted)
 (type &keys :salted salted)
 (type number)
 (type)
 :defaults ((number 10) (salted t)))

I assume that what happens is that it tries to match the first which structurally fits, right? In which case I see where the problem with the keys matching occurs and why it is a problem.

I think we could have a "stricter" version of &plist which would first check with plist-member and if the key exist accept nil as valid binding so that it passes -when-let... only if the binding does not exist it will fail.

alphapapa commented 6 years ago

but you are using -when-let which fails if any binding is nil, that's probably the issue.

Ah, yes, I see. It seemed like the most natural -let-like alternative to pcase, but I guess it's not quite that simple.

Is there any reason why you have multiple signatures

Well, yes, the point is that the function can accept arguments in any of those formats. cl-defun can't handle a signature like (&optional type (number 10) &key (salted t)), which seems like the natural way to describe it. So being able to specify a signature in -let style would be very useful.

I assume that what happens is that it tries to match the first which structurally fits, right? In which case I see where the problem with the keys matching occurs and why it is a problem.

I don't think that structurally fits. The arg list is ('peanuts :salted nil), which should match the second form, (type &keys :salted salted), right?

I think we could have a "stricter" version of &plist which would first check with plist-member and if the key exist accept nil as valid binding so that it passes -when-let... only if the binding does not exist it will fail.

Yes, that would be great! Would you be interested in implementing that? If not, do you have any pointers? I don't know the Dash internals the way you do...

Fuco1 commented 6 years ago

About the stricter version, that actually might be quite difficult, because the nil check is handled in -if-let not in the internal dash--match function which does the expansion. And -if-let just checks for non nil and that's it.

(I was thinking in dash--match we could add calls to plist-match or similar but that wouldn't help at all)

alphapapa commented 6 years ago

LOL, at the very moment I finished expanding the series of macros and arrived at:

(let
    ((salted
      (when
          (plist-member --dash-source-267-- :salted)
        (plist-get --dash-source-267-- :salted))))
  (if salted
      (progn
        (funcall g1383))))

...I see your message that's way ahead of me. :)

alphapapa commented 6 years ago

If we could make it expand to something like this, maybe that would work? But I don't know where to begin implementing that.

(let ((--dash-source-267-- args))
  (if --dash-source-267--
      (let ((type (pop --dash-source-267--)))
        (if type
            (let ((number (pop --dash-source-267--))
                  (salted-set-p (plist-member --dash-source-267-- :salted)))
              (if (and number
                       salted-set-p)
                  (let ((salted (plist-get --dash-source-267-- :salted)))
                    (progn
                      (funcall g1383)))))))))
alphapapa commented 6 years ago

Well, not exactly on the topic of Dash, but I found a way to implement it with pcase:

(defun plist-p (list)
  (when list
    (cl-loop for (keyword value) on list by #'cddr
             always (keywordp keyword))))

(let ((args '((peanuts :salted nil)
              (peanuts 20 :salted nil)
              (peanuts 20)
              (peanuts))))
  (cl-loop for set in args
           collect (a-list :args set
                           :result (let ((number 10)
                                         (salted t))
                                     (pcase set
                                       (`(,type . ,(and rest
                                                        (guard (plist-p rest))
                                                        (let (map salted) rest)))
                                        (list :type type :number number :salted salted))
                                       (`(,type ,number . ,(and rest
                                                                (guard (plist-p rest))
                                                                (let (map salted) rest)))
                                        (list :type type :number number :salted salted))
                                       (`(,type ,number)
                                        (list :type type :number number :salted salted))
                                       (`(,type)
                                        (list :type type :number number :salted salted))
                                       (_ (user-error "Invalid args")))))))
;; =>
;; (((:args peanuts :salted nil)
;;   (:result :type peanuts :number 10 :salted nil))
;;  ((:args peanuts 20 :salted nil)
;;   (:result :type peanuts :number 20 :salted nil))
;;  ((:args peanuts 20)
;;   (:result :type peanuts :number 20 :salted t))
;;  ((:args peanuts)
;;   (:result :type peanuts :number 10 :salted t)))

Now the issue is transforming a list like:

((type number &keys :salted salted)
 (type &keys :salted salted)
 (type number)
 (type)
 :defaults ((number 10) (salted t)))

Into the expansion. :)

basil-conto commented 6 years ago

Sorry, I haven't really been following this thread, but just a quick question: Are plist keywords required to satisfy keywordp under the existing and proposed plist destructuring schemes for dash.el?

alphapapa commented 6 years ago

@basil-conto Are you talking about https://github.com/magnars/dash.el/pull/269?

basil-conto commented 6 years ago

@alphapapa I'm asking how opinionated dash.el is on the type of plist keywords, as a general design principle and possibly as an existing restriction in the code.

AFAICT from a quick glance, the PRs relevant to this question are #81, #111, #268, and #269. I haven't looked at -let recently enough to remember off the top of my head its relevant semantics.

alphapapa commented 6 years ago

@basil-conto Your question may be answered by https://github.com/magnars/dash.el/issues/111#issuecomment-404137467 (at least, with regard to that PR).

basil-conto commented 6 years ago

@alphapapa

Your question may be answered by https://github.com/magnars/dash.el/issues/111#issuecomment-404137467

That comment, as well as the following quick test run of #269:


(-let [(&plist :foo x) '(:foo 1 foo 2 x 3)] x) ; => 1
(-let [(&plist 'foo x) '(:foo 1 foo 2 x 3)] x) ; => 2
(-let [(&plist :foo) '(:foo 1 foo 2 x 3)] foo) ; => 1
(-let [(&plist 'foo) '(:foo 1 foo 2 x 3)] foo) ; => 2

both indicate that -let is just using plist-get (as opposed to lax-plist-get or some custom lookup function) for plist destructuring, so any key type under eq will work.

(at least, with regard to that PR).

What prompted my initial question, however, was your definition of plist-p here: https://github.com/magnars/dash.el/issues/268#issuecomment-404195296

This suggests that only plists with keywordp keys will be accepted. Is that so, and if so, why?

alphapapa commented 6 years ago

What prompted my initial question, however, was your definition of plist-p here

My definition of that function has nothing to do with Dash. :)

basil-conto commented 6 years ago

What prompted my initial question, however, was your definition of plist-p here

My definition of that function has nothing to do with Dash. :)

OK, thanks for clarifying, and sorry about the noise.

alphapapa commented 6 years ago

@Fuco1 Do you have any general advice for building lists containing backquotes and unquotes? Or alternatives to doing that in the first place? As you can see in https://github.com/magnars/dash.el/issues/268#issuecomment-404195296, I figured out a way to make a pcase that works, but creating that pcase programatically seems very difficult due to inserting backquotes and unquotes. After much trial-and-error, I made some progress, but even then I couldn't seem to create a pattern with a dot in it. Maybe there's something obvious I'm missing...

Fuco1 commented 6 years ago

@alphapapa Not really no, I struggle with those myself. They are handled in quite annoying way.

I think it would still be better to maybe invent a notation where we could have just one arg declaration. Ideally -defun would work like -lambda in that it would use the destructuring system of -let.

alphapapa commented 6 years ago

I think it would still be better to maybe invent a notation where we could have just one arg declaration.

Sorry, I don't understand what you mean.

Ideally -defun would work like -lambda in that it would use the destructuring system of -let.

Yes, definitely.

Fuco1 commented 6 years ago

@alphapapa Well right now you have

((type number &keys :salted salted)
 (type &keys :salted salted)
 (type number)
 (type)
 :defaults ((number 10) (salted t)))

which is not aligned with -lambda currently. Also I like to follow the convention that adding - prefix to the built-in versions should keep everything working exactly the same so that any lambda can be converted to -lambda without any error.

yyoncho commented 5 years ago

@alphapapa @Fuco1 what is the status of this PR? Can we add -defun which works exactly like -lambda as per @Fuco1's commend and eventually something like --defun(?) for what @alphapapa is suggesting?

Luis-Henriquez-Perez commented 5 years ago

I'm not sure if this helps but, I've written a -defun macro that's based on -lambda by mostly just copying the code from the -lambda. It seems to work.

(defmacro -defun (name match-form &rest body)
  "Macro that allows desctructuring for defun."
  (declare (doc-string 3)
           (indent defun)
           (debug (&define sexp
                           [&optional stringp]
                           [&optional ("interactive" interactive)]
                           def-body)))
  (let* ((docstring (when (stringp (car-safe body)) (car body)))
         (body (if docstring (cdr body) body)))
    (cond
     ((not (consp match-form))
      (signal 'wrong-type-argument "match-form must be a list"))
     ((-all? 'symbolp match-form)
      `(defun ,name ,match-form ,docstring ,@body))
     (t
      (let* ((inputs (--map-indexed (list it (make-symbol (format "input%d" it-index)))
                                    match-form)))
        `(defun ,name ,(--map (cadr it) inputs) ,docstring 
                (-let* ,inputs ,@body)))))))

Also, I would like a more detailed specification of how the pattern matching suggested by @alphapapa will work so that I (and others seeing this issue) could potentially start working on it.

Right now I have a general idea of what it will do, but there are some use cases where I am unsure what the result will be.

For example one question I have is: how will the -defun determine what to do with an argument list like:

((one two three) (four five) (six))

This could be an example of patten matching suggested by alphapapa where -defun can have 3, 2, or 1 argument. However, it could also be a destructuring match form where the function expects 3 arguments: a list of length 3, a list of length 2 and a list of length one.

Another question is should -defun also have optional type checking? I think that'd be nice to distinguish cases where arguments are of the same length. Though I'm unsure of what syntax should be used to do this.

ericdallo commented 4 years ago

https://github.com/magnars/dash.el/pull/336