bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode

[Alan Mackenzie <acm@muc.de>]

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 <jporterbugs@gmail.com>
> >> 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).