From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Drew Adams Newsgroups: gmane.emacs.bugs Subject: bug#16483: 24.3.50; `read-face-name' is a mess now (regression) Date: Fri, 17 Jan 2014 15:54:11 -0800 (PST) Message-ID: <55d774e6-27f4-4e9f-a3e5-4aa1e43f0a0e@default> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1390002903 23345 80.91.229.3 (17 Jan 2014 23:55:03 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 17 Jan 2014 23:55:03 +0000 (UTC) To: 16483@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Jan 18 00:55:08 2014 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1W4JFk-00030M-AE for geb-bug-gnu-emacs@m.gmane.org; Sat, 18 Jan 2014 00:55:08 +0100 Original-Received: from localhost ([::1]:40689 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4JFj-0000P7-Ki for geb-bug-gnu-emacs@m.gmane.org; Fri, 17 Jan 2014 18:55:07 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:38995) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4JFg-0000OV-CZ for bug-gnu-emacs@gnu.org; Fri, 17 Jan 2014 18:55:04 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W4JFf-0002G3-PQ for bug-gnu-emacs@gnu.org; Fri, 17 Jan 2014 18:55:04 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:41597) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4JFf-0002EB-JV for bug-gnu-emacs@gnu.org; Fri, 17 Jan 2014 18:55:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1W4JFe-0008L8-IG for bug-gnu-emacs@gnu.org; Fri, 17 Jan 2014 18:55:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Drew Adams Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 17 Jan 2014 23:55:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 16483 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.139000288732030 (code B ref -1); Fri, 17 Jan 2014 23:55:01 +0000 Original-Received: (at submit) by debbugs.gnu.org; 17 Jan 2014 23:54:47 +0000 Original-Received: from localhost ([127.0.0.1]:55616 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1W4JFO-0008KX-Hk for submit@debbugs.gnu.org; Fri, 17 Jan 2014 18:54:47 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:37114) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1W4JFL-0008KP-N6 for submit@debbugs.gnu.org; Fri, 17 Jan 2014 18:54:44 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W4JFB-00023F-Bu for submit@debbugs.gnu.org; Fri, 17 Jan 2014 18:54:43 -0500 Original-Received: from lists.gnu.org ([2001:4830:134:3::11]:50425) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4JFB-00023B-89 for submit@debbugs.gnu.org; Fri, 17 Jan 2014 18:54:33 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:38674) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4JF2-00084v-JO for bug-gnu-emacs@gnu.org; Fri, 17 Jan 2014 18:54:33 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W4JEt-0001yv-KJ for bug-gnu-emacs@gnu.org; Fri, 17 Jan 2014 18:54:24 -0500 Original-Received: from aserp1040.oracle.com ([141.146.126.69]:49578) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W4JEt-0001yr-EO for bug-gnu-emacs@gnu.org; Fri, 17 Jan 2014 18:54:15 -0500 Original-Received: from acsinet21.oracle.com (acsinet21.oracle.com [141.146.126.237]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id s0HNsDcA018911 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 17 Jan 2014 23:54:14 GMT Original-Received: from userz7022.oracle.com (userz7022.oracle.com [156.151.31.86]) by acsinet21.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id s0HNsClS003375 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 17 Jan 2014 23:54:13 GMT Original-Received: from abhmp0008.oracle.com (abhmp0008.oracle.com [141.146.116.14]) by userz7022.oracle.com (8.14.5+Sun/8.14.4) with ESMTP id s0HNsCGT009214 for ; Fri, 17 Jan 2014 23:54:12 GMT X-Priority: 3 X-Mailer: Oracle Beehive Extensions for Outlook 2.0.1.8 (707110) [OL 12.0.6680.5000 (x86)] X-Source-IP: acsinet21.oracle.com [141.146.126.237] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Spam-Score: -4.0 (----) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-Spam-Score: -4.0 (----) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:83672 Archived-At: The doc string is a mess now. Quite unclear, and incorrect in more than one way. And so is the code a mess. 1. The doc string no longer says what the behavior is if DEFAULT is nil. In Emacs 24.3, it said this for that case: If DEFAULT is nil, the list of default face names is taken from the `read-face-name' property of the text at point, or, if that is nil, from the `face' property of the text at point. What happens now? No information about that. See below for my guess. Please tell users of the function what its behavior is. 2. The doc string says that DEFAULT is returned if the user enters the empty string. That is clearly wrong, at least when DEFAULT is a list of faces or face names. DEFAULT is not returned in such cases. The most that can be said in general is that DEFAULT _determines_ what is returned for empty input - not that DEFAULT _is_ what is returned. 3. A list of symbols is not a list of face _names_. A face is not a face name. Especially for functions like this, the doc should clearly distinguish faces (symbols) from face names (strings). All the more so, because what is allowed and required as input for this function has changed multiple times across Emacs versions. 4. The doc string does not allow for DEFAULT to be a face name, but that case is supported by the code. (And if it were not supported then this would be another incompatible change from previous Emacs versions.) If it should not be supported then raise an error in that case. If it should be supported then correct the doc string. 5. The code is wrong, at least in this regard: If DEFAULT is a list of strings (face names), and if MULTIPLE is nil, both of which are OK per the doc string, then you get this: Debugger entered--Lisp error: (wrong-type-argument symbolp "font-lock-comme= =3D nt-face") * symbol-name("font-lock-comment-face") * (cond ((symbolp default) (symbol-name default)) (multiple (mapconcat (fun= =3D ction (lambda (f) (if (symbolp f) (symbol-name f) f))) default ", ")) (t (s= =3D ymbol-name (car default)))) * (setq default (cond ((symbolp default) (symbol-name default)) (multiple (= =3D mapconcat (function (lambda (f) (if (symbolp f) (symbol-name f) f))) defaul= =3D t ", ")) (t (symbol-name (car default))))) * (if (and default (not (stringp default))) (setq default (cond ((symbolp d= =3D efault) (symbol-name default)) (multiple (mapconcat (function (lambda (f) (= =3D if ... ... f))) default ", ")) (t (symbol-name (car default)))))) ... * read-face-name("face: " ("font-lock-comment-face" "highlight") nil) 6. If raising an error for #5 is the correct behavior and the doc is wrong about allowing DEFAULT to be a list of face names, then here is a proposed doc correction, assuming I understand the behavior right (which is not sure= ): If non-nil, DEFAULT should be a face (a symbol), a face name (a string) or a list of faces (symbols). DEFAULT determines what is returned if the user just hits `RET' (empty input), as follows: If DEFAULT is nil then return nil. If DEFAULT is a single face, then return its name. If DEFAULT is a list of faces, then: If MULTIPLE is nil, return the name of the first face in the list. If MULTIPLE is non-nil, return DEFAULT. 7. The doc string should not mention `completing-read-multiple' or the separator regexp. Anyway, the code uses the value of `crm-separator'; it does not use the literal regexp mentioned in the doc. The doc should just say something like this (to be appended to that suggested in #6): If MULTIPLE is non-nil, read multiple face names and return them as a list. If MULTIPLE is nil, read and return a single face name. 8. What happened to the useful defaulting of previous Emacs versions? Yes, I know why you made the change, but now any existing code that uses `read-face-name' is broken if it depends on `r-f-n' to provide such defaulting. Too bad. Please consider: There is more Elisp code in the world than just what is distributed by Emacs Dev. `read-face-name' has been and continues to be a poster child of how not to evolve code. It has morphed in incompatible ways from version to version. The right way to do what you wanted to do for Emacs 24.4 would have been to go ahead and define `face-at-point', but to *use* it in `read-face-name', so that that function continues to provide the expected defaulting when DEFAULT is nil: (unless default (setq default (face-at-point))) I somehow doubt that that part of the regression will be fixed, but perhaps some of the other points above have a chance of being addressed. In GNU Emacs 24.3.50.1 (i686-pc-mingw32) of 2014-01-07 on ODIEONE Bzr revision: 115916 bzg@gnu.org-20140107233629-du2solx6tmxnx0np Windowing system distributor `Microsoft Corp.', version 6.1.7601 Configured using: `configure --prefix=3D3D/c/Devel/emacs/binary --enable-checking=3D3Dyes,gl= yphs 'CFLAGS=3D3D-O0 -g3' LDFLAGS=3D3D-Lc:/Devel/emacs/lib CPPFLAGS=3D3D-Ic:/Devel/emacs/include'