From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: "Garreau\, Alexandre" Newsgroups: gmane.emacs.devel Subject: Re: Replace trivial pcase occurrences in the Emacs sources Date: Tue, 30 Oct 2018 02:15:56 +0100 Message-ID: <87y3agp73n.fsf@portable.galex-713.eu> References: <83tvlcsnee.fsf@gnu.org> <86mur137n8.fsf@gmail.com> <20181029130132.GB4195@ACM> <20181029134722.GC4195@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: blaine.gmane.org 1540862055 19645 195.159.176.226 (30 Oct 2018 01:14:15 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 30 Oct 2018 01:14:15 +0000 (UTC) User-Agent: Gnus (5.13), GNU Emacs 25.1.1 (i686-pc-linux-gnu, GTK+ Version 3.22.11) of 2017-09-15, modified by Debian Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Oct 30 02:14:11 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gHIbg-0004wt-VC for ged-emacs-devel@m.gmane.org; Tue, 30 Oct 2018 02:14:09 +0100 Original-Received: from localhost ([::1]:50000 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gHIdn-0003VU-A8 for ged-emacs-devel@m.gmane.org; Mon, 29 Oct 2018 21:16:19 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:42754) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gHIdY-0003Tm-4M for emacs-devel@gnu.org; Mon, 29 Oct 2018 21:16:06 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gHIdW-00040e-M8 for emacs-devel@gnu.org; Mon, 29 Oct 2018 21:16:04 -0400 Original-Received: from portable.galex-713.eu ([2a00:5884:8305::1]:59630) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gHIdU-0003zI-TY for emacs-devel@gnu.org; Mon, 29 Oct 2018 21:16:02 -0400 Original-Received: from localhost ([::1] helo=portable.galex-713.eu) by portable.galex-713.eu with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1gHIdQ-0008Vk-Et; Tue, 30 Oct 2018 02:15:56 +0100 PGP-FINGERPRINT: E109 9988 4197 D7CB B0BC 5C23 8DEB 24BA 867D 3F7F Accept-Language: fr, en, eo, it, br In-Reply-To: (Stefan Monnier's message of "Mon, 29 Oct 2018 17:08:38 -0400") X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:5884:8305::1 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:230791 Archived-At: On 2018-10-29 at 17:08, Stefan Monnier wrote: >>> > Doc strings which specify fully the arguments to these macros, includ= ing >>> > their semantics, and say what the macros do. The current doc strings >>> > (at least some of them) for these macros don't do this. >>> I understand this in theory, but I don't know what it means in this >>> concrete case. >> Take a look at, for example, the doc string for pcase-dolist. In its >> entirety, it's this: > > Thanks. What do you think of the patch below? > > I'd rather keep it defined in terms of its differences w.r.t `dolist`, > but if really needed, we could change the doc so it doesn't rely on > `dolist`s own doc at all. You can do that while being more explicit on how arguments relate. A little redundancy might be welcome, but I believe the standard minimum (that, for instance, would satisfy checkdoc) would only to cite each argument name in upcase and say it is like in `dolist', only if this is *exactely the case*. It is not for =E2=80=9CPATTERN=E2=80=9D so usage ough= t to be more detailed: instead of describing what is done, you=E2=80=99d better explain = what it is for. So in =E2=80=9CMore specifically `dolist's VAR binding is repla= ced by a PATTERN against which each element of the list is matched.=E2=80=9D yo= u=E2=80=99re simply describing your macro implementation, which readers doesn=E2=80=99t = want (otherwise they would be reading function=E2=80=99s source), you=E2=80=99d = better say =E2=80=9CPATTERN allow to bind several variables at once through a `pcase' pattern, instead of only one var as in simple `dolist'=E2=80=9D. This is w= ay clearer (I really ought to learn how to make up patches ><). > We do have to keep the reference to `pcase` because we don't want to > repeat the definition of what a pcase pattern can look like. Of course. But you want to precise how exactely does it relate to `pcase'. > (defmacro pcase-let (bindings &rest body) > "Like `let' but where you can use `pcase' patterns for bindings. > BODY should be a list of expressions, and BINDINGS should be a list of b= indings > -of the form (PAT EXP). > +of the form (PATTERN EXP). So `pcase-let' doesn=E2=80=99t support single variable names bound to `nil'= (not that sad, I (sadly) never use that anyway, that=E2=80=99s only for lexical locality tweaking)? I checked, in fact it indeed doesn=E2=80=99t: knowing = the other `pcase-' functions docstrings tendency to omit optional behavior, if I didn=E2=80=99t checked, I=E2=80=99d have bet it were supporting it. Then, contrarily to `let' you might make your docstring more readable by changing the arglist spec: "\(fn ((PATTERN EXP)...) BODY...)" or you may be more formal, so it=E2=80=99d become more readable: =E2=80=9CB= INDINGS is a list of the form (BINDING...), with each BINDING is (PATTERN EXP), where PATTERN is a `pcase' pattern, and EXP the expression it must match.=E2=80=9D > ;;;###autoload > (defmacro pcase-dolist (spec &rest body) > - "Like `dolist' but where the binding can be a `pcase' pattern. > + "Superset of `dolist' where the VAR binding can be a `pcase' PATTERN. > +More specifically `dolist's VAR binding is replaced by a PATTERN > +against which each element of the list is matched. > +As in the case of `pcase-let', PATTERN is matched under the assumption > +that it *will* match. First I don=E2=80=99t believe it is a good idea to talk about `pcase-let' i= n the docstring of another function, or then you should cite the whole family (including `pcase-lambda', etc.), except if they=E2=80=99re tightly tied, b= ut then you should also cite `pcase-dolist' in `pcase-let' docstring, so to make a followable dual-link. > The macro is expanded and optimized under the assumption that those > patterns *will* match, so a mismatch may go undetected or may cause > any kind of error." That=E2=80=99s not explicit enough on the result if not doing so: what kind= of error? >From what I see there: #+BEGIN_SRC emacs-lisp (pcase-let ((`(,a ,b ,c) (list 1 2))) (list a b c)) ; =3D> (1 2 nil) (pcase-let ((`(,a ,b (2 3)) (list 1 2 3))) (list a b)) ;; =3D> (wrong-type-argument listp 3) (pcase-let ((`(,a ,b (2 3)) (list 1 2 (list 4 5)))) (list a b)) ;; =3D> (1 2) #+END_SRC It seems to be like `pcase-lambda': it binds nil, and doesn=E2=80=99t err anything, as long as the structures are in the same places and the same types. This behavior ought to be documented, or changed. In my message about `pcase-lambda', I wrote: > A given subpattern inside PATTERN might not match values (equality) or > even types, except for each of its subsequences, which *have* to > exist, have the right type, and be in the right place. > \n(fn (PATTERN LIST) BODY...)" > (declare (indent 1) (debug ((pcase-PAT form) body))) > (if (pcase--trivial-upat-p (car spec)) So `pcase-dolist' doesn=E2=80=99t support `dolist'=E2=80=99s `result'? I b= et it does (if it doesn=E2=80=99t it=E2=80=99s sad), and like in `pcase-lambda' you ag= ain omitted optional arguments from your docstring: this is confusing, as you might as well have implemented it so that to remove or override these.