From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode Date: Thu, 16 Sep 2021 20:49:20 +0000 Message-ID: References: <021853bf-0169-c158-ab3d-296b6c144e08@gmail.com> <83r1dufgxu.fsf@gnu.org> <94c7b4ec-813b-515f-d947-116c294dd74b@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="2475"; mail-complaints-to="usenet@ciao.gmane.io" Cc: acm@muc.de, 50538@debbugs.gnu.org To: Jim Porter Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Sep 16 22:50:18 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mQyKo-0000QT-5i for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 16 Sep 2021 22:50:18 +0200 Original-Received: from localhost ([::1]:49844 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQyKm-0001EP-Nt for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 16 Sep 2021 16:50:16 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:46460) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQyKX-0001CO-V6 for bug-gnu-emacs@gnu.org; Thu, 16 Sep 2021 16:50:01 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:45167) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mQyKX-0003kg-MM for bug-gnu-emacs@gnu.org; Thu, 16 Sep 2021 16:50:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mQyKX-00072E-KZ for bug-gnu-emacs@gnu.org; Thu, 16 Sep 2021 16:50:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 16 Sep 2021 20:50:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 50538 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 50538-submit@debbugs.gnu.org id=B50538.163182537626996 (code B ref 50538); Thu, 16 Sep 2021 20:50:01 +0000 Original-Received: (at 50538) by debbugs.gnu.org; 16 Sep 2021 20:49:36 +0000 Original-Received: from localhost ([127.0.0.1]:56713 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mQyK8-00071L-8t for submit@debbugs.gnu.org; Thu, 16 Sep 2021 16:49:36 -0400 Original-Received: from colin.muc.de ([193.149.48.1]:23990 helo=mail.muc.de) by debbugs.gnu.org with smtp (Exim 4.84_2) (envelope-from ) id 1mQyJz-00070s-QZ for 50538@debbugs.gnu.org; Thu, 16 Sep 2021 16:49:34 -0400 Original-Received: (qmail 9023 invoked by uid 3782); 16 Sep 2021 20:49:20 -0000 Original-Received: from acm.muc.de (p4fe15a47.dip0.t-ipconnect.de [79.225.90.71]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Thu, 16 Sep 2021 22:49:20 +0200 Original-Received: (qmail 9539 invoked by uid 1000); 16 Sep 2021 20:49:20 -0000 Content-Disposition: inline In-Reply-To: <94c7b4ec-813b-515f-d947-116c294dd74b@gmail.com> X-Submission-Agent: TMDA/1.3.x (Ph3nix) X-Primary-Address: acm@muc.de X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:214507 Archived-At: Hello, Jim. On Sun, Sep 12, 2021 at 11:05:17 -0700, Jim Porter wrote: > On 9/11/2021 11:26 PM, Eli Zaretskii wrote: > >> From: Jim Porter > >> Date: Sat, 11 Sep 2021 20:58:47 -0700 > >> There are a few related issues with pairing double quotes in CC mode > >> while using `electric-pair-mode'. Hopefully the steps to reproduce below > >> will explain the issues. In all the cases, I'd expect > >> `electric-pair-mode' to insert a closing quote, but it doesn't. OK. Just for context, I'm the maintainer of CC Mode, but I don't use electric-pair-mode in my day-to-day editing. > > Your expected results seem to expect Emacs to assume that a new string > > will be inserted, but is that an assumption that is always true? > In these cases, I believe that's true (with the default > `electric-pair-mode' settings, that is). More broadly, the goal of the > patch is to ensure that pairing of double quotes works the same in CC > mode as it does in other modes (`ruby-mode', `python-mode', `js-mode', > `emacs-lisp-mode', etc), which is why I added `c-mode' to the list of > modes to test in `test/lisp/electric-tests.el'. The goal should be to make all these modes work correctly with respect to e-p-m, for whatever value of correctly we decide upon. > That said, there's one potential case I didn't account for (mostly > because it wasn't accounted for in the patch for bug#36474): if a user > customizes `electric-pair-inhibit-predicate' to inhibit cases like (2) > or (3) in my original message, that won't work right in CC modes, since > the default value of `electric-pair-inhibit-predicate' (set by the user) > won't be called. > Attached is an updated patch that changes the logic of > `c-electric-pair-inhibit-predicate' to either a) inhibit insertion of > the closing quote, or b) call the default-value of > `electric-pair-inhibit-predicate' to determine what to do. This should > give users more consistent behavior when customizing > `electric-pair-inhibit-predicate'. There were two or three minor problems with the patch: 1-/. CC Mode doesn't use syntax-ppss at all. This was because way back when, syntax-ppss was buggy, and even now doesn't do the right thing for CC Mode in some edge cases (e.g. with the buffer narrowed and point-min inside a string or comment). Instead it uses its own internal syntactic cacheing, largely centred around the function c-semi-pp-to-literal. 2/- Rather than using get-text-property and friends directly, CC Mode uses the macros c-get-char-property, etc. This is (?was) to maintain compatibility with XEmacs. 3/- (A bit more serious) The patch looks for the last " in the current line without taking account of any escaped new lines. There is already a CC Mode macro which does all the work here, c-point, which can be given the argument 'eoll for "end of logical line". Incorporating all these points into the macro (which I admit I haven't tested) the function would look something like this: ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Connection with Emacs's electric-pair-mode (defun c-electric-pair-inhibit-predicate (char) "Return t to inhibit the insertion of a second copy of CHAR. At the time of call, point is just after the newly inserted CHAR. When CHAR is \" and not within a comment, t will be returned if the quotes on the current line are already balanced (i.e. if the last \" is not marked with a string fence syntax-table text property). For other cases, the default value of `electric-pair-inhibit-predicate' is called and its value returned. This function is the appropriate value of `electric-pair-inhibit-predicate' for CC Mode modes, which mark invalid strings with such a syntax table text property on the opening \" and the next unescaped end of line." (or (and (eq char ?\") (not (memq (cadr (c-semi-pp-to-literal (1- (point)))) '(c c++))) (not (equal (c-get-char-property (c-point 'eoll) 'c-fl-syn-tab) '(15)))) (funcall (default-value 'electric-pair-inhibit-predicate) char))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; > The tests still pass, although I wasn't able to figure out a good way to > add a test for setting `electric-pair-inhibit-predicate' that works with > how CC mode overrides it (using `:bindings' in > `define-electric-pair-test' didn't work, since the binding is set too > late). Hopefully that's ok; if not, I can try and see about making some > more extensive changes to the tests to account for this. > Note however that this solution isn't perfect: it means a user's custom > `electric-pair-inhibit-predicate' can only inhibit *more* than CC mode's > default behavior, not less. I think that's a reasonable compromise > though, and users who want more direct control can set > `electric-pair-inhibit-predicate' inside `c-mode-common-hook'. A > "perfect" solution here would probably require adding new customization > points to `electric-pair-mode' (e.g. a way for major modes to override > how the syntax is analyzed), and I'm not sure the added complexity would > be worth it, especially since this code is already a bit tricky. Indeed so. ;-) -- Alan Mackenzie (Nuremberg, Germany).