emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Replace trivial pcase occurrences in the Emacs sources


From: Eli Zaretskii
Subject: Re: Replace trivial pcase occurrences in the Emacs sources
Date: Wed, 24 Oct 2018 18:03:26 +0300

> From: Stefan Monnier <address@hidden>
> Date: Tue, 23 Oct 2018 14:12:34 -0400
> 
> > I think so too, but it seems the majority of developers has a different
> > option, at least in this (old) thread.
> 
> So I guess I'm gonna have to try and argue my case.
> OK, here are the advantages I see for `cl-case`:

FWIW, my main problem is not even with cl-case, it's with 'cond'.  It
seems like we have some 'pcase' epidemic, whose first symptom is that
people stop using 'cond' and start using 'pcase' instead.  A few
examples:

todo-mode.el:

    (pcase this-param
      ('edit (todo-edit-item--text))
      ('header (todo-edit-item--text 'include-header))
      ('multiline (todo-edit-item--text 'multiline))
      ('add/edit (todo-edit-item--text 'comment-edit))
      ('delete (todo-edit-item--text 'comment-delete))
      ('diary (todo-edit-item--diary-inclusion))
      ('nonmarking (todo-edit-item--diary-inclusion 'nonmarking))
      [...]

auth-source-pass.el:

    (pcase (length matching-entries)
      (0 (auth-source-pass--do-debug "no match found")
         nil)
      (1 (auth-source-pass--do-debug "found 1 match: %s" (car matching-entries))
         (car matching-entries))
      (_ (auth-source-pass--select-one-entry matching-entries user)))))

bs.el:

           (setq bs-buffer-show-mark (pcase bs-buffer-show-mark
                                       (`nil   'never)
                                       (`never 'always)
                                       (_       nil))))))

calculator.el:

               (<= inp (pcase calculator-input-radix
                         (`nil ?9) (`bin ?1) (`oct ?7) (_ 999))))

lpr.el:

                 (pcase (count-lines (point-min) (point-max))
                   (0 "")
                   (1 ": ")
                   (_ ":\n"))

My favorite is imap.el, which does something like the above in 3
nested levels.  I will spare you the code, you can look it up.

Is it because people are too lazy to write (eq a b) as part of 'cond'?
Or is there something else I'm missing?  

You may wonder why I'm bothered by such uses.  It's because people are
evidently confused by 'pcase's arcane syntax, and therefore produce
obfuscated code even in the simple usage.  For example:

apropos.el:

      (pcase (car-safe x)
        ;; (autoload (push (cdr x) autoloads))
        (`require (push (cdr x) requires))
        (`provide (push (cdr x) provides))
        (`t nil) ; Skip "was an autoload" entries.
        ;; FIXME: Print information about each individual method: both
        ;; its docstring and specializers (bug#21422).
        (`cl-defmethod (push (cadr x) provides))
        (_ (push (or (cdr-safe x) x) symbols))))

(Quick: what's the difference between `require and 'require in this
case?)

easy-mmode.el:

      (pcase keyw
        (`:group (setq group (nconc group (list :group (pop keys)))))
        (`:global (setq keys (cdr keys)))
        (_ (push keyw extra-keywords) (push (pop keys) extra-keywords))))

(Aren't keywords supposed to be self-quoting? then why they are
explicitly quoted?)

We have dozens of such fragments in our codebase, which just makes the
sources harder to read, especially for people who aren't fluent with
'pcase' (which seems to be the case with many of us).

So I think we should begin by rewriting all of such uses as simple
'cond', and ask contributors not to use 'pcase' where a simple 'cond'
will do.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]