From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Noam Postavsky Newsgroups: gmane.emacs.bugs Subject: bug#32939: 27.0.50; Don't reject confusable quotes, just fontify & mention them in error messages Date: Mon, 29 Oct 2018 21:30:43 -0400 Message-ID: <87woq0i5ks.fsf@gmail.com> References: <87lg7dmfhp.fsf@gmail.com> <83woqw235n.fsf@gnu.org> <877ei7m172.fsf@gmail.com> <838t2joh34.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1540862962 25298 195.159.176.226 (30 Oct 2018 01:29:22 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 30 Oct 2018 01:29:22 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) Cc: 32939@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Oct 30 02:29:18 2018 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1gHIqI-0006OI-Pr for geb-bug-gnu-emacs@m.gmane.org; Tue, 30 Oct 2018 02:29:15 +0100 Original-Received: from localhost ([::1]:50083 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gHIsO-0002j9-JG for geb-bug-gnu-emacs@m.gmane.org; Mon, 29 Oct 2018 21:31:24 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:46389) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gHIs7-0002gc-6M for bug-gnu-emacs@gnu.org; Mon, 29 Oct 2018 21:31:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gHIs2-0002SP-0r for bug-gnu-emacs@gnu.org; Mon, 29 Oct 2018 21:31:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:48191) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gHIs1-0002SK-RZ for bug-gnu-emacs@gnu.org; Mon, 29 Oct 2018 21:31:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1gHIs1-0003FB-NB for bug-gnu-emacs@gnu.org; Mon, 29 Oct 2018 21:31:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Noam Postavsky Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 30 Oct 2018 01:31:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 32939 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 32939-submit@debbugs.gnu.org id=B32939.154086305311950 (code B ref 32939); Tue, 30 Oct 2018 01:31:01 +0000 Original-Received: (at 32939) by debbugs.gnu.org; 30 Oct 2018 01:30:53 +0000 Original-Received: from localhost ([127.0.0.1]:52449 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gHIrs-000360-Io for submit@debbugs.gnu.org; Mon, 29 Oct 2018 21:30:53 -0400 Original-Received: from mail-it1-f171.google.com ([209.85.166.171]:54408) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gHIrq-0002zz-Rr for 32939@debbugs.gnu.org; Mon, 29 Oct 2018 21:30:51 -0400 Original-Received: by mail-it1-f171.google.com with SMTP id d6so6357909itl.4 for <32939@debbugs.gnu.org>; Mon, 29 Oct 2018 18:30:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=NLTc1dTvHWjBjtfxHG2nK8EB/+GHm/Qxr9F3M1QtO0k=; b=VQsc3bzQpzEsIO5tUPvCC3WY34IXPBvoiTlzwA4rNvJAil9MjPDYvs8R1Vm5SXw3mZ AhijKjNUD6OVd+ItIefqD78PeKr54nDs4fcm2zpXts++MV+Y7HN6kf6eSoQX3hqflJGp PJ1W8tryfF6UhAcACEptJfkoDt7730En50Jm0edoHs86EKIEe697qSf2wweDrMsxZdNS Oesrh5gD691CtiAFjmmag44rpgMVRlls1O9Wjq/7VBipuC5oQD2amazfVpK4vWAF2oqs rUiEwFz0qeaWbgsNb+AOODMMHQm8VGYc395ErgU/RXiAFihtZbRK2ccsvEGYni6z7dbQ 7KpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=NLTc1dTvHWjBjtfxHG2nK8EB/+GHm/Qxr9F3M1QtO0k=; b=bDJJRn44uFXkSMtwj8WHoMjzFbUX+sLtrG5kwBziowiFhLMAntl0iN5tkx/AmlYUCT 2X7s0dsE5twgLrxmdWaJrSQUokcVPbcJFQeYWZ86pbJaJa2Jod0FaNWOq2shTllas8sy e/ovC6B5tLnRHX5zmMHnutDOVM45qaAzaFuQOrJnC5Rj7izbqoV03T7BtQ8K/eZwrRKf 2IzJFkWHrWxQstPZPNpsvxLOnRGYZbcti7ADsdsWbPE3JTRqEE6ccrtrrolLvYXIsD5U MUQ9vBRbrgs6dQtexrnl5YXnaHqyrumswkqBPCHixO+FZ+He006b5zswzSSc0ZV05mrS Q0mA== X-Gm-Message-State: AGRZ1gJVNMQcgP0W++BlqZgiKB0tSF6OpumJRzPA8DK4McJRWkZwJAsl NOAGN0Xydzr3O8ph0iMU/Faef3MV X-Google-Smtp-Source: AJdET5c+szGPPdW3wacJk5NYSmX0Opd0fgOt47lOW5M4SIg4pXWbizSB/Ft0J3NssEmOTsgF/tWBiQ== X-Received: by 2002:a24:1351:: with SMTP id 78-v6mr145228itz.42.1540863045110; Mon, 29 Oct 2018 18:30:45 -0700 (PDT) Original-Received: from zebian (cbl-45-2-119-34.yyz.frontiernetworks.ca. [45.2.119.34]) by smtp.googlemail.com with ESMTPSA id g8-v6sm2017306iof.11.2018.10.29.18.30.44 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 29 Oct 2018 18:30:44 -0700 (PDT) In-Reply-To: <838t2joh34.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 27 Oct 2018 12:48:47 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.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" Xref: news.gmane.org gmane.emacs.bugs:151799 Archived-At: --=-=-= Content-Type: text/plain Eli Zaretskii writes: > However, perhaps we should mention this behavior in "Basic Char > Syntax" and the related use of this face in "Faces for Font Lock". > WDYT? I've tried adding some mentions of it in those places (unfilled to simplify diff). Though I'm not sure if it makes sense to talk about this somewhat obscure case. >> -(ert-deftest lread-tests--funny-quote-symbols () > Should we replace this test by one that tests the fontification? Right, good point, added. And I had missed disabling fontification for backslash escaped characters, now fixed. --=-=-= Content-Type: text/plain; charset=utf-8 Content-Disposition: attachment; filename=v3-0002-Improve-errors-warnings-due-to-fancy-quoted-vars-.patch Content-Transfer-Encoding: quoted-printable Content-Description: patch >From b19dbd60af4282ab8dcd1512f6910da6671d8a4a Mon Sep 17 00:00:00 2001 From: Noam Postavsky Date: Sat, 10 Mar 2018 18:12:55 -0500 Subject: [PATCH v3 2/2] Improve errors & warnings due to fancy quoted vars (Bug#32939) Add some hints to the message for byte compiler free & unused variable warnings, and 'void-variable' errors where the variable has confusable quote characters in it. * lisp/help.el (uni-confusables), uni-confusables-regexp): New constants. (help-command-error-confusable-suggestions): New function, added to `command-error-function'. (help-uni-confusable-suggestions): New function. * lisp/emacs-lisp/bytecomp.el (byte-compile-variable-ref): * lisp/emacs-lisp/cconv.el (cconv--analyze-use): Use it. * lisp/emacs-lisp/lisp-mode.el (lisp--match-confusable-symbol-character): New function. (lisp-fdefs): Use it to fontify confusable characters with font-lock-warning-face when they occur in symbol names. * doc/lispref/modes.texi (Faces for Font Lock): * doc/lispref/objects.texi (Basic Char Syntax): Recommend backslash escaping of confusable characters, and mention new fontification. * etc/NEWS: Announce the new fontification behavior. * test/lisp/emacs-lisp/lisp-mode-tests.el (lisp-fontify-confusables): New test. --- doc/lispref/modes.texi | 3 +- doc/lispref/objects.texi | 7 ++++- etc/NEWS | 4 +++ lisp/emacs-lisp/bytecomp.el | 10 +++++-- lisp/emacs-lisp/cconv.el | 6 ++-- lisp/emacs-lisp/lisp-mode.el | 18 +++++++++++- lisp/help.el | 49 +++++++++++++++++++++++++++++= ++++ test/lisp/emacs-lisp/lisp-mode-tests.el | 24 ++++++++++++++++ 8 files changed, 114 insertions(+), 7 deletions(-) diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi index 49b7e1ea3f..243e535988 100644 --- a/doc/lispref/modes.texi +++ b/doc/lispref/modes.texi @@ -3190,7 +3190,8 @@ Faces for Font Lock @table @code @item font-lock-warning-face @vindex font-lock-warning-face -for a construct that is peculiar, or that greatly changes the meaning of +for a construct that is peculiar (e.g., an unescaped confusable quote +in an Emacs Lisp symbol like @samp{=E2=80=98foo}), or that greatly changes= the meaning of other text, like @samp{;;;###autoload} in Emacs Lisp and @samp{#error} in C. =20 diff --git a/doc/lispref/objects.texi b/doc/lispref/objects.texi index a0940032ee..f82ee7c49a 100644 --- a/doc/lispref/objects.texi +++ b/doc/lispref/objects.texi @@ -347,7 +347,12 @@ Basic Char Syntax characters. However, you must add a backslash before any of the characters @samp{()[]\;"}, and you should add a backslash before any of the characters @samp{|'`#.,} to avoid confusing the Emacs commands -for editing Lisp code. You can also add a backslash before whitespace +for editing Lisp code. You should also add a backslash before Unicode +characters which resemble the previously mentioned @acronym{ASCII} +ones, to avoid confusing people reading your code. Emacs will +highlight some non-escaped commonly confused characters such as +@samp{=E2=80=98} to encourage this. + You can also add a backslash before whitespace characters such as space, tab, newline and formfeed. However, it is cleaner to use one of the easily readable escape sequences, such as @samp{\t} or @samp{\s}, instead of an actual whitespace character such diff --git a/etc/NEWS b/etc/NEWS index 3530e04467..395169253d 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1089,6 +1089,10 @@ and if the new behavior breaks your code please email 32252@debbugs.gnu.org. Because %o and %x can now format signed integers, they now support the + and space flags. =20 +** In Emacs Lisp mode, symbols with confusable quotes are highlighted. +For example, the first character in '=E2=80=98foo' would be highlighted in +'font-lock-warning-face'. + +++ ** Omitting variables after '&optional' and '&rest' is now allowed. For example (defun foo (&optional)) is no longer an error. This is diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 8b67e1007d..31d589ab02 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -3407,7 +3407,10 @@ byte-compile-variable-ref (boundp var) (memq var byte-compile-bound-variables) (memq var byte-compile-free-references)) - (byte-compile-warn "reference to free variable `%S'" var) + (let* ((varname (prin1-to-string var)) + (suggestions (help-uni-confusable-suggestions varname))) + (byte-compile-warn "reference to free variable `%s'%s" varname + (if suggestions (concat "\n " suggestions) "= "))) (push var byte-compile-free-references)) (byte-compile-dynamic-variable-op 'byte-varref var)))) =20 @@ -3423,7 +3426,10 @@ byte-compile-variable-set (boundp var) (memq var byte-compile-bound-variables) (memq var byte-compile-free-assignments)) - (byte-compile-warn "assignment to free variable `%s'" var) + (let* ((varname (prin1-to-string var)) + (suggestions (help-uni-confusable-suggestions varname))) + (byte-compile-warn "assignment to free variable `%s'%s" varname + (if suggestions (concat "\n " suggestions) "= "))) (push var byte-compile-free-assignments)) (byte-compile-dynamic-variable-op 'byte-varset var)))) =20 diff --git a/lisp/emacs-lisp/cconv.el b/lisp/emacs-lisp/cconv.el index 010026b416..b62c69868b 100644 --- a/lisp/emacs-lisp/cconv.el +++ b/lisp/emacs-lisp/cconv.el @@ -591,8 +591,10 @@ cconv--analyze-use (eq ?_ (aref (symbol-name var) 0)) ;; As a special exception, ignore "ignore". (eq var 'ignored)) - (byte-compile-warn "Unused lexical %s `%S'" - varkind var))) + (let ((suggestions (help-uni-confusable-suggestions (symbol-name va= r)))) + (byte-compile-warn "Unused lexical %s `%S'%s" + varkind var + (if suggestions (concat "\n " suggestions) ""= ))))) ;; If it's unused, there's no point converting it into a cons-cell, ev= en if ;; it's captured and mutated. (`(,binder ,_ t t ,_) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index f31b4d4dd2..619d525309 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -280,6 +280,19 @@ elisp--font-lock-backslash `(face ,font-lock-warning-face help-echo "This \\ has no effect")))) =20 +(defun lisp--match-confusable-symbol-character (limit) + ;; Match a confusable character within a Lisp symbol. + (catch 'matched + (while t + (if (re-search-forward uni-confusables-regexp limit t) + ;; Skip confusables which are backslash escaped, or inside + ;; strings or comments. + (save-match-data + (unless (or (eq (char-before (match-beginning 0)) ?\\) + (nth 8 (syntax-ppss))) + (throw 'matched t))) + (throw 'matched nil))))) + (let-when-compile ((lisp-fdefs '("defmacro" "defun")) (lisp-vdefs '("defvar")) @@ -463,7 +476,10 @@ elisp--font-lock-backslash (3 'font-lock-regexp-grouping-construct prepend)) (lisp--match-hidden-arg (0 '(face font-lock-warning-face - help-echo "Hidden behind deeper element; move to another li= ne?"))) + help-echo "Hidden behind deeper element; move to anoth= er line?"))) + (lisp--match-confusable-symbol-character + 0 '(face font-lock-warning-face + help-echo "Confusable character")) )) "Gaudy level highlighting for Emacs Lisp mode.") =20 diff --git a/lisp/help.el b/lisp/help.el index 6e06fc9e1c..2cc28fca24 100644 --- a/lisp/help.el +++ b/lisp/help.el @@ -1489,6 +1489,55 @@ help--make-usage-docstring (help--docstring-quote (format "%S" (help--make-usage fn arglist))))) =20 + +;; Just some quote-like characters for now. TODO: generate this stuff +;; from official Unicode data. +(defconst uni-confusables + '((#x2018 . "'") ;; LEFT SINGLE QUOTATION MARK + (#x2019 . "'") ;; RIGHT SINGLE QUOTATION MARK + (#x201B . "'") ;; SINGLE HIGH-REVERSED-9 QUOTATION MARK + (#x201C . "\"") ;; LEFT DOUBLE QUOTATION MARK + (#x201D . "\"") ;; RIGHT DOUBLE QUOTATION MARK + (#x201F . "\"") ;; DOUBLE HIGH-REVERSED-9 QUOTATION MARK + (#x301E . "\"") ;; DOUBLE PRIME QUOTATION MARK + (#xFF02 . "'") ;; FULLWIDTH QUOTATION MARK + (#xFF07 . "'") ;; FULLWIDTH APOSTROPHE + )) + +(defconst uni-confusables-regexp + (concat "[" (mapcar #'car uni-confusables) "]")) + +(defun help-uni-confusable-suggestions (string) + "Return a message describing confusables in STRING." + (let ((i 0) + (confusables nil)) + (while (setq i (string-match uni-confusables-regexp string i)) + (let ((replacement (alist-get (aref string i) uni-confusables))) + (push (aref string i) confusables) + (setq string (replace-match replacement t t string)) + (setq i (+ i (length replacement))))) + (when confusables + (format-message + (if (> (length confusables) 1) + "Found confusable characters: %s; perhaps you meant: `%s'?" + "Found confusable character: %s, perhaps you meant: `%s'?") + (mapconcat (lambda (c) (format-message "`%c'" c)) + confusables ", ") + string)))) + +(defun help-command-error-confusable-suggestions (data _context _signal) + (pcase data + (`(void-variable ,var) + (let ((suggestions (help-uni-confusable-suggestions + (symbol-name var)))) + (when suggestions + (princ (concat "\n " suggestions) t)))) + (_ nil))) + +(add-function :after command-error-function + #'help-command-error-confusable-suggestions) + + (provide 'help) =20 ;;; help.el ends here diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp= /lisp-mode-tests.el index 30f606d381..8339591740 100644 --- a/test/lisp/emacs-lisp/lisp-mode-tests.el +++ b/test/lisp/emacs-lisp/lisp-mode-tests.el @@ -20,6 +20,10 @@ (require 'ert) (require 'cl-lib) (require 'lisp-mode) +(require 'faceup) + + +;;; Indentation =20 (defconst lisp-mode-tests--correctly-indented-sexp "\ \(a @@ -256,7 +260,27 @@ lisp-mode-tests--correctly-indented-sexp (lisp-indent-line) (should (equal (buffer-string) "prompt> foo")))) =20 + +;;; Fontification =20 +(ert-deftest lisp-fontify-confusables () + "Unescaped 'smart quotes' should be fontified in `font-lock-warning-face= '." + (with-temp-buffer + (dolist (ch + '(#x2018 ;; LEFT SINGLE QUOTATION MARK + #x2019 ;; RIGHT SINGLE QUOTATION MARK + #x201B ;; SINGLE HIGH-REVERSED-9 QUOTATION MARK + #x201C ;; LEFT DOUBLE QUOTATION MARK + #x201D ;; RIGHT DOUBLE QUOTATION MARK + #x201F ;; DOUBLE HIGH-REVERSED-9 QUOTATION MARK + #x301E ;; DOUBLE PRIME QUOTATION MARK + #xFF02 ;; FULLWIDTH QUOTATION MARK + #xFF07 ;; FULLWIDTH APOSTROPHE + )) + (insert (format "=C2=ABw:%c=C2=BBfoo \\%cfoo\n" ch ch))) + (let ((faceup (buffer-string))) + (faceup-clean-buffer) + (should (faceup-test-font-lock-buffer 'emacs-lisp-mode faceup))))) =20 (provide 'lisp-mode-tests) ;;; lisp-mode-tests.el ends here --=20 2.11.0 --=-=-=--