* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
@ 2021-09-12 3:58 Jim Porter
2021-09-12 6:26 ` Eli Zaretskii
0 siblings, 1 reply; 24+ messages in thread
From: Jim Porter @ 2021-09-12 3:58 UTC (permalink / raw)
To: 50538
[-- Attachment #1: Type: text/plain, Size: 2302 bytes --]
(Note: I've just updated my copyright assignment information, but
haven't received confirmation that everything is in order, so this might
need to wait until that's done for it to merge.)
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. You can
try similar steps in a `ruby-mode' buffer to see how it should work.
----------------------------------------
Common setup
------------
$ cat foo.c
"foobar"
$ emacs -Q foo.c
M-x electric-pair-mode
Note that | represents the point below.
1. Quote pairing in comments
----------------------------
C-o ;; to make a blank line
// " ;; type this
Expected: line 1 is // "|"
Actual: line 1 is // "|
2. Inserting quote pairs before existing string
-----------------------------------------------
" ;; type this (point is at beginning of buffer, before "foobar")
Expected: line 1 is "|""foobar"
Actual: line 1 is "|"foobar"
3. Splitting strings into two
-----------------------------
"foo|bar" ;; move point here
" ;; type this
Expected: line 1 is "foo"|"bar"
Actual: line 1 is "foo"|bar"
----------------------------------------
This is because the logic in the patch for bug#36474 isn't quite right.
Currently, `c-electric-pair-inhibit-predicate' checks if the
newly-inserted quotation mark has "a string fence syntax-table text
property" (i.e. if it's the start of a string literal not terminated on
that line[1]). However, this fails in all three cases above: in (1)
because we're in a comment, not a string literal; and in (2) and (3)
because it's the *last* quotation mark on the line that's unterminated,
not the one before point.
The attached patch fixes this by taking those cases into account. I also
added `c-mode' to the list of modes to check in
`test/lisp/electric-tests.el'. This required setting single-line
comments as the default in those tests, since the tests expect
single-line comments (I tried testing under `c++-mode', but the tests
failed, I think due to <> being paren-like in C++).
[1] I think this is what it means, at least (or close to it).
[-- Attachment #2: 0001-Improve-behavior-of-electric-pair-mode-in-cc-mode.patch --]
[-- Type: text/plain, Size: 3189 bytes --]
From d9c8879b3082a40375552ddb629c83a0d941bff9 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 29 Aug 2021 12:40:26 -0700
Subject: [PATCH] Improve behavior of 'electric-pair-mode' in 'cc-mode'
This ensures that quotes are paired correctly within comments, allows for
insertion of quote pairs immediately before another quote, and allows
inserting quote pairs within a string (thus splitting the string in two).
* lisp/progmodes/cc-mode.el (c-electric-pair-inhibit-predicate):
Inhibit insertion of paired quote in fewer cases.
* test/lisp/electric-tests.el (define-electric-pair-test):
Add 'c-mode' to list of modes to test by default.
---
lisp/progmodes/cc-mode.el | 19 +++++++++++++------
test/lisp/electric-tests.el | 5 ++++-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 057d292246..e2e50efa63 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -2550,17 +2550,24 @@ c-electric-pair-inhibit-predicate
At the time of call, point is just after the newly inserted CHAR.
-When CHAR is \", t will be returned unless the \" is marked with
-a string fence syntax-table text property. For other characters,
-the default value of `electric-pair-inhibit-predicate' is called
-and its value returned.
+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."
- (if (eq char ?\")
- (not (equal (get-text-property (1- (point)) 'c-fl-syn-tab) '(15)))
+ (if (and (eq char ?\")
+ (not (nth 4 (save-excursion (syntax-ppss (1- (point)))))))
+ (let ((last-quote (save-match-data
+ (save-excursion
+ (search-forward "\n" nil 'move)
+ (search-backward "\"" nil)))))
+ (not (equal (get-text-property last-quote 'c-fl-syn-tab) '(15))))
(funcall (default-value 'electric-pair-inhibit-predicate) char)))
\f
diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 666de89c53..35516a9f0b 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -32,6 +32,9 @@
(require 'elec-pair)
(require 'cl-lib)
+;; When running tests in c-mode, use single-line comments (//).
+(add-hook 'c-mode-hook (lambda () (c-toggle-comment-style -1)))
+
(defun call-with-saved-electric-modes (fn)
(let ((saved-electric (if electric-pair-mode 1 -1))
(saved-layout (if electric-layout-mode 1 -1))
@@ -173,7 +176,7 @@ define-electric-pair-test
expected-string
expected-point
bindings
- (modes '(quote (ruby-mode js-mode)))
+ (modes '(quote (ruby-mode js-mode c-mode)))
(test-in-comments t)
(test-in-strings t)
(test-in-code t)
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-12 3:58 bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode Jim Porter
@ 2021-09-12 6:26 ` Eli Zaretskii
2021-09-12 18:05 ` Jim Porter
0 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-09-12 6:26 UTC (permalink / raw)
To: Jim Porter; +Cc: 50538
> 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.
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? It
could be that the user wants to modify the existing string instead, in
which case your suggested patches will require the user to delete more
quotes than previously.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-12 6:26 ` Eli Zaretskii
@ 2021-09-12 18:05 ` Jim Porter
2021-09-15 22:17 ` Jim Porter
2021-09-16 20:49 ` Alan Mackenzie
0 siblings, 2 replies; 24+ messages in thread
From: Jim Porter @ 2021-09-12 18:05 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 50538
[-- Attachment #1: Type: text/plain, Size: 2677 bytes --]
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.
>
> 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'.
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'.
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.
[-- Attachment #2: 0001-Improve-behavior-of-electric-pair-mode-in-cc-mode.patch --]
[-- Type: text/plain, Size: 3286 bytes --]
From 0b038ef868a8c558e66d420b9380b792b409e388 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 12 Sep 2021 10:44:55 -0700
Subject: [PATCH] Improve behavior of 'electric-pair-mode' in 'cc-mode'
This ensures that quotes are paired correctly within comments, allows for
insertion of quote pairs immediately before another quote, and allows
inserting quote pairs within a string (thus splitting the string in two).
* lisp/progmodes/cc-mode.el (c-electric-pair-inhibit-predicate):
Inhibit insertion of paired quote in fewer cases.
* test/lisp/electric-tests.el (define-electric-pair-test):
Add 'c-mode' to list of modes to test by default.
---
lisp/progmodes/cc-mode.el | 21 ++++++++++++++-------
test/lisp/electric-tests.el | 5 ++++-
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 057d292246..012488a7b1 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -2550,18 +2550,25 @@ c-electric-pair-inhibit-predicate
At the time of call, point is just after the newly inserted CHAR.
-When CHAR is \", t will be returned unless the \" is marked with
-a string fence syntax-table text property. For other characters,
-the default value of `electric-pair-inhibit-predicate' is called
-and its value returned.
+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."
- (if (eq char ?\")
- (not (equal (get-text-property (1- (point)) 'c-fl-syn-tab) '(15)))
- (funcall (default-value 'electric-pair-inhibit-predicate) char)))
+ (or (and (eq char ?\")
+ (not (nth 4 (save-excursion (syntax-ppss (1- (point))))))
+ (let ((last-quote (save-match-data
+ (save-excursion
+ (search-forward "\n" nil 'move)
+ (search-backward "\"" nil)))))
+ (not (equal (get-text-property last-quote 'c-fl-syn-tab) '(15)))))
+ (funcall (default-value 'electric-pair-inhibit-predicate) char)))
\f
;; Support for C
diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 666de89c53..35516a9f0b 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -32,6 +32,9 @@
(require 'elec-pair)
(require 'cl-lib)
+;; When running tests in c-mode, use single-line comments (//).
+(add-hook 'c-mode-hook (lambda () (c-toggle-comment-style -1)))
+
(defun call-with-saved-electric-modes (fn)
(let ((saved-electric (if electric-pair-mode 1 -1))
(saved-layout (if electric-layout-mode 1 -1))
@@ -173,7 +176,7 @@ define-electric-pair-test
expected-string
expected-point
bindings
- (modes '(quote (ruby-mode js-mode)))
+ (modes '(quote (ruby-mode js-mode c-mode)))
(test-in-comments t)
(test-in-strings t)
(test-in-code t)
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-12 18:05 ` Jim Porter
@ 2021-09-15 22:17 ` Jim Porter
2021-09-16 5:25 ` Eli Zaretskii
2021-09-16 20:49 ` Alan Mackenzie
1 sibling, 1 reply; 24+ messages in thread
From: Jim Porter @ 2021-09-15 22:17 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 50538
On 9/11/2021 8:58 PM, Jim Porter wrote:
> (Note: I've just updated my copyright assignment information, but
> haven't received confirmation that everything is in order, so this might
> need to wait until that's done for it to merge.)
I've gotten confirmation that my copyright assignment info is all
up-to-date, so once this patch passes muster, it should be ok to merge it.
On 9/12/2021 11:05 AM, Jim Porter wrote:
> 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.
I'm not sure if someone has a better idea for how to do things, but for
my config[1], the patch works well and makes CC modes behave the same as
other programming modes. In my opinion, the worst thing
`electric-pair-mode' can do is to behave inconsistently, since that
forces the user to pay close attention to something that should be
almost invisible/automatic.
[1] I customize `electric-pair-inhibit-predicate' to disable
electric-pairing in strings/comments, and this patch interacts correctly
with that customization.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-15 22:17 ` Jim Porter
@ 2021-09-16 5:25 ` Eli Zaretskii
2021-09-16 12:40 ` Lars Ingebrigtsen
2021-09-16 18:26 ` Alan Mackenzie
0 siblings, 2 replies; 24+ messages in thread
From: Eli Zaretskii @ 2021-09-16 5:25 UTC (permalink / raw)
To: Jim Porter, Lars Ingebrigtsen, Alan Mackenzie; +Cc: 50538
> From: Jim Porter <jporterbugs@gmail.com>
> Cc: 50538@debbugs.gnu.org
> Date: Wed, 15 Sep 2021 15:17:23 -0700
>
> On 9/11/2021 8:58 PM, Jim Porter wrote:
> > (Note: I've just updated my copyright assignment information, but
> > haven't received confirmation that everything is in order, so this might
> > need to wait until that's done for it to merge.)
>
> I've gotten confirmation that my copyright assignment info is all
> up-to-date, so once this patch passes muster, it should be ok to merge it.
Your copyright assignment is on file, so we are good in that
department.
> On 9/12/2021 11:05 AM, Jim Porter wrote:
> > 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.
>
> I'm not sure if someone has a better idea for how to do things, but for
> my config[1], the patch works well and makes CC modes behave the same as
> other programming modes. In my opinion, the worst thing
> `electric-pair-mode' can do is to behave inconsistently, since that
> forces the user to pay close attention to something that should be
> almost invisible/automatic.
I don't use this minor mode, so I don't think my opinion on this
matters much. I will defer to Lars and Alan here.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 5:25 ` Eli Zaretskii
@ 2021-09-16 12:40 ` Lars Ingebrigtsen
2021-09-16 12:59 ` Dmitry Gutov
2021-09-16 18:26 ` Alan Mackenzie
1 sibling, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-16 12:40 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Jim Porter, 50538, Alan Mackenzie
Eli Zaretskii <eliz@gnu.org> writes:
> I don't use this minor mode, so I don't think my opinion on this
> matters much. I will defer to Lars and Alan here.
I seldom use electric-pair-mode, and since this touches cc-mode, I'll
defer to Alan. :-)
Alan?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 12:40 ` Lars Ingebrigtsen
@ 2021-09-16 12:59 ` Dmitry Gutov
2021-09-16 13:17 ` Lars Ingebrigtsen
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Gutov @ 2021-09-16 12:59 UTC (permalink / raw)
To: Lars Ingebrigtsen, Eli Zaretskii; +Cc: Jim Porter, 50538, Alan Mackenzie
On 16.09.2021 15:40, Lars Ingebrigtsen wrote:
> Eli Zaretskii<eliz@gnu.org> writes:
>
>> I don't use this minor mode, so I don't think my opinion on this
>> matters much. I will defer to Lars and Alan here.
> I seldom use electric-pair-mode, and since this touches cc-mode, I'll
> defer to Alan.:-)
>
> Alan?
At risk of reigniting an old disagreement, perhaps we should ask Joao?
IIUC, Alan doesn't really use electric-pair-mode.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 12:59 ` Dmitry Gutov
@ 2021-09-16 13:17 ` Lars Ingebrigtsen
2021-09-16 17:04 ` João Távora
0 siblings, 1 reply; 24+ messages in thread
From: Lars Ingebrigtsen @ 2021-09-16 13:17 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Jim Porter, João Távora, 50538, Alan Mackenzie
Dmitry Gutov <dgutov@yandex.ru> writes:
>>> I don't use this minor mode, so I don't think my opinion on this
>>> matters much. I will defer to Lars and Alan here.
>> I seldom use electric-pair-mode, and since this touches cc-mode, I'll
>> defer to Alan.:-)
>> Alan?
>
> At risk of reigniting an old disagreement, perhaps we should ask Joao?
Sure; added to the CCs. João, do you have an opinion here?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 13:17 ` Lars Ingebrigtsen
@ 2021-09-16 17:04 ` João Távora
2021-09-16 17:11 ` Eli Zaretskii
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: João Távora @ 2021-09-16 17:04 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: Jim Porter, Dmitry Gutov, 50538, Alan Mackenzie
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>
>>>> I don't use this minor mode, so I don't think my opinion on this
>>>> matters much. I will defer to Lars and Alan here.
>>> I seldom use electric-pair-mode, and since this touches cc-mode, I'll
>>> defer to Alan.:-)
>>> Alan?
>>
>> At risk of reigniting an old disagreement, perhaps we should ask Joao?
>
> Sure; added to the CCs. João, do you have an opinion here?
I couldn't understand the initial issue fully, but I can only confirm
that the relationship between cc-mode and electric-pair-mode has been
rocky. I think the situation is similar with electric-layout-mode and
with electric-indent-mode, to a certain degree (though I am not sure for
this last one).
I don't think there are any easy technical solutions for it. I certainly
cannot invest the effort in any of them right now.
It wasn't like this from the beginning: when I first created
electric-pair-mode it worked mostly if not fully fine with cc-mode, like
one expects e-p-m to work in other modes (ruby, python, js, elisp, rust,
perl, even non-programming modes).
At times, compatibility deteriorated as cc-mode added mechanisms for
solving electricity problems or related problems in an idiosyncratic
way. Sometimes these changes broke e-p-m's tests and the solution was
-- incorrectly in my view --to disable the tests "temporarily". I see
that for one of them at least, the test has been re-instated. A good
sign, maybe.
Regardless of history and current state of affairs, my personal solution
to C in Emacs is to use a hand-rolled mode, a so-called plainer-cc-mode.
The goal is to make it behave like most other modes w.r.t
electric-pair-mode and keep the basic c syntax and indentation. I've
used it reasonably successfully with C and C++. Here it is in its
entirety. I think I use something similar for c++-mode.
(define-derived-mode plainer-c-mode c-mode "pC"
"A plainer C-mode with no internal electric machinery."
(c-toggle-electric-state -1)
(setq-local electric-indent-local-mode-hook nil)
(setq-local electric-indent-mode-hook nil)
(electric-indent-local-mode 1)
(dolist (key '(?\" ?\' ?\{ ?\} ?\( ?\) ?\[ ?\]))
(local-set-key (vector key) 'self-insert-command)))
Among other simpler things, this makes the major mode not bind special
keys to special commands. They are all bound to `self-insert-command`
like most major modes. This simplifies electric-pair-mode, as far as I
remember.
The above is a hack, but not very dirty one. The one below much more so.
It's a brute-force hack for solving electricity problems. It simply
disables some cc internal functions. May be out of date by now, YMMV,
warranty void, etc, as they say.
(defun joaot/disable-some-cc-things ()
(interactive)
(dolist (name '(c-restore-string-fences
c-remove-string-fences
c-before-change-check-unbalanced-strings
c-after-change-mark-abnormal-strings
c-after-change-escape-NL-in-string))
(advice-add name :override #'ignore '((name . joaot/remove-disable-some-cc-things)))
(add-hook 'c-mode-common-hook
(lambda ()
(kill-local-variable 'electric-pair-inhibit-predicate)))))
On the C++/C editing-front, I am eagerly waiting for Treesitter to
arrive so that is it becomes possible to write a new major mode for c++
and c from scratch. Reusing solid and efficient parsing logic based on
language grammars. Another more closely available option, is to simply
use LSP for indentation and fontification. Though that is almost
certainly slower and less confortable/portable/etc... Anyway, whatever
the solution, I'm quite confident that such a mode won't have these
characteristics that cc-mode has when used with `electric-pair-mode`.
If you're writing C, and not C++, my also try Stefan Monnier's aptly
named sm-c-mode, installable via M-x package-install RET sm-c-mode.
Works fine with electric-pair-mode, but seems to indent very oddly for
some reason.
Hope this helps. I'm sorry I can't offer any better solutions.
João
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 17:04 ` João Távora
@ 2021-09-16 17:11 ` Eli Zaretskii
2021-09-16 17:33 ` João Távora
2021-09-16 17:29 ` Jim Porter
2021-09-16 19:05 ` Alan Mackenzie
2 siblings, 1 reply; 24+ messages in thread
From: Eli Zaretskii @ 2021-09-16 17:11 UTC (permalink / raw)
To: João Távora; +Cc: jporterbugs, larsi, dgutov, 50538, acm
> From: João Távora <joaotavora@gmail.com>
> Cc: Dmitry Gutov <dgutov@yandex.ru>, Eli Zaretskii <eliz@gnu.org>, Jim
> Porter <jporterbugs@gmail.com>, 50538@debbugs.gnu.org, Alan Mackenzie
> <acm@muc.de>
> Date: Thu, 16 Sep 2021 18:04:53 +0100
>
> Hope this helps. I'm sorry I can't offer any better solutions.
Thanks, but I'm not sure I understand what is your opinion about the
proposed patch. Are you saying we shouldn't install it?
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 17:04 ` João Távora
2021-09-16 17:11 ` Eli Zaretskii
@ 2021-09-16 17:29 ` Jim Porter
2021-09-16 19:05 ` Alan Mackenzie
2 siblings, 0 replies; 24+ messages in thread
From: Jim Porter @ 2021-09-16 17:29 UTC (permalink / raw)
To: João Távora, Lars Ingebrigtsen
Cc: Alan Mackenzie, 50538, Dmitry Gutov
On 9/16/2021 10:04 AM, João Távora wrote:
>
> I couldn't understand the initial issue fully, but I can only confirm
> that the relationship between cc-mode and electric-pair-mode has been
> rocky. I think the situation is similar with electric-layout-mode and
> with electric-indent-mode, to a certain degree (though I am not sure for
> this last one).
Hopefully the following summary will help. My patch is essentially an
enhancement of the patch from bug#36474. In that bug, Alan Mackenzie
describes the problem:
> Diagnosis: electric-pair--unbalanced-strings-p works after the (single)
> newly typed " has been stripped from the buffer. It attempts to
> determine whether there are any open strings after the point of
> insertion. It does this by using parse-partial-sexp, and checks (nth 3
> <result>) as evidence of an open string.
>
> This does not work in CC Mode, since although there is an open string
> marker (with a string fence syntax-table property on it) this is
> "closed" (from parse-partial-sexp's point of view) by the string fence
> property on the newline at the end of the line.
> electric-pair--unbalanced-strings-p thus returns the wrong result.
The fix in that bug was to check if the just-inserted double-quote "is
marked with a string fence syntax-table text property". That fixes the
issue described in bug#36474, but it's not quite the right logic. CC
Mode gives the *last* double-quote on a line the string fence property
if a line has unbalanced quotes. Thus, the patch changes the behavior to
check the last double-quote on the line, rather than the just-inserted
double-quote.
The patch makes one other improvement as well: it doesn't check for the
string fence property on a double-quote inside a comment. CC Mode
doesn't apply string fence properties there, since it's not necessary.
Therefore, inside a comment, `c-electric-pair-inhibit-predicate' just
defers to the default value of `electric-pair-inhibit-predicate'.
As I mentioned earlier in the thread, this isn't quite perfect behavior,
but it significantly improves the common case (`electric-pair-mode' with
default or "default-like" settings). CC Mode's practice of closing
strings at the end of a line - even without a closing quote - just
doesn't play nicely with `electric-pair-mode', so barring some major
changes to either CC Mode or `electric-pair-mode', I can't think of a
way to improve this patch beyond where it's at now.
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 17:11 ` Eli Zaretskii
@ 2021-09-16 17:33 ` João Távora
0 siblings, 0 replies; 24+ messages in thread
From: João Távora @ 2021-09-16 17:33 UTC (permalink / raw)
To: Eli Zaretskii
Cc: Jim Porter, Lars Ingebrigtsen, Dmitry Gutov, 50538,
Alan Mackenzie
On Thu, Sep 16, 2021 at 6:11 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Cc: Dmitry Gutov <dgutov@yandex.ru>, Eli Zaretskii <eliz@gnu.org>, Jim
> > Porter <jporterbugs@gmail.com>, 50538@debbugs.gnu.org, Alan Mackenzie
> > <acm@muc.de>
> > Date: Thu, 16 Sep 2021 18:04:53 +0100
> >
> > Hope this helps. I'm sorry I can't offer any better solutions.
>
> Thanks, but I'm not sure I understand what is your opinion about the
> proposed patch. Are you saying we shouldn't install it?
That patch targets mainly cc-mode.el. To have an opinion would
require me to understand it, interpret its meaning. That has many
implications... I can't do that right now. I can confirm that the small
change to electric-tests.el (which activates many tests for c-mode) is
indeed the way I wrote those tests originally. Maybe Jim has
found a silver bullet?
João
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 5:25 ` Eli Zaretskii
2021-09-16 12:40 ` Lars Ingebrigtsen
@ 2021-09-16 18:26 ` Alan Mackenzie
1 sibling, 0 replies; 24+ messages in thread
From: Alan Mackenzie @ 2021-09-16 18:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Jim Porter, Lars Ingebrigtsen, 50538
Hello, Eli.
On Thu, Sep 16, 2021 at 08:25:33 +0300, Eli Zaretskii wrote:
> > From: Jim Porter <jporterbugs@gmail.com>
> > Cc: 50538@debbugs.gnu.org
> > Date: Wed, 15 Sep 2021 15:17:23 -0700
> > On 9/11/2021 8:58 PM, Jim Porter wrote:
> > > (Note: I've just updated my copyright assignment information, but
> > > haven't received confirmation that everything is in order, so this might
> > > need to wait until that's done for it to merge.)
> > I've gotten confirmation that my copyright assignment info is all
> > up-to-date, so once this patch passes muster, it should be ok to merge it.
> Your copyright assignment is on file, so we are good in that
> department.
> > On 9/12/2021 11:05 AM, Jim Porter wrote:
> > > 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.
> > I'm not sure if someone has a better idea for how to do things, but for
> > my config[1], the patch works well and makes CC modes behave the same as
> > other programming modes. In my opinion, the worst thing
> > `electric-pair-mode' can do is to behave inconsistently, since that
> > forces the user to pay close attention to something that should be
> > almost invisible/automatic.
> I don't use this minor mode, so I don't think my opinion on this
> matters much. I will defer to Lars and Alan here.
Thanks, I've downloaded the bug thread from debbugs, and I'll look at
it. I don't actually use the minor mode either, but the patch is to do
with C++ Mode.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 17:04 ` João Távora
2021-09-16 17:11 ` Eli Zaretskii
2021-09-16 17:29 ` Jim Porter
@ 2021-09-16 19:05 ` Alan Mackenzie
2021-09-16 20:51 ` João Távora
2 siblings, 1 reply; 24+ messages in thread
From: Alan Mackenzie @ 2021-09-16 19:05 UTC (permalink / raw)
To: João Távora; +Cc: Jim Porter, Lars Ingebrigtsen, 50538, Dmitry Gutov
Hello, João.
On Thu, Sep 16, 2021 at 18:04:53 +0100, João Távora wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> > Dmitry Gutov <dgutov@yandex.ru> writes:
> >>>> I don't use this minor mode, so I don't think my opinion on this
> >>>> matters much. I will defer to Lars and Alan here.
> >>> I seldom use electric-pair-mode, and since this touches cc-mode, I'll
> >>> defer to Alan.:-)
> >>> Alan?
> >> At risk of reigniting an old disagreement, perhaps we should ask Joao?
> > Sure; added to the CCs. João, do you have an opinion here?
> I couldn't understand the initial issue fully, but I can only confirm
> that the relationship between cc-mode and electric-pair-mode has been
> rocky. I think the situation is similar with electric-layout-mode and
> with electric-indent-mode, to a certain degree (though I am not sure for
> this last one).
I think it is up to both of us to make this relationship less rocky.
> I don't think there are any easy technical solutions for it. I certainly
> cannot invest the effort in any of them right now.
> It wasn't like this from the beginning: when I first created
> electric-pair-mode it worked mostly if not fully fine with cc-mode, like
> one expects e-p-m to work in other modes (ruby, python, js, elisp, rust,
> perl, even non-programming modes).
> At times, compatibility deteriorated as cc-mode added mechanisms for
> solving electricity problems or related problems in an idiosyncratic
> way. Sometimes these changes broke e-p-m's tests and the solution was
> -- incorrectly in my view --to disable the tests "temporarily". I see
> that for one of them at least, the test has been re-instated. A good
> sign, maybe.
> Regardless of history and current state of affairs, my personal solution
> to C in Emacs is to use a hand-rolled mode, a so-called plainer-cc-mode.
> The goal is to make it behave like most other modes w.r.t
> electric-pair-mode and keep the basic c syntax and indentation. I've
> used it reasonably successfully with C and C++. Here it is in its
> entirety. I think I use something similar for c++-mode.
> (define-derived-mode plainer-c-mode c-mode "pC"
> "A plainer C-mode with no internal electric machinery."
> (c-toggle-electric-state -1)
> (setq-local electric-indent-local-mode-hook nil)
> (setq-local electric-indent-mode-hook nil)
> (electric-indent-local-mode 1)
> (dolist (key '(?\" ?\' ?\{ ?\} ?\( ?\) ?\[ ?\]))
> (local-set-key (vector key) 'self-insert-command)))
> Among other simpler things, this makes the major mode not bind special
> keys to special commands. They are all bound to `self-insert-command`
> like most major modes. This simplifies electric-pair-mode, as far as I
> remember.
> The above is a hack, but not very dirty one. The one below much more so.
> It's a brute-force hack for solving electricity problems. It simply
> disables some cc internal functions. May be out of date by now, YMMV,
> warranty void, etc, as they say.
> (defun joaot/disable-some-cc-things ()
> (interactive)
> (dolist (name '(c-restore-string-fences
> c-remove-string-fences
> c-before-change-check-unbalanced-strings
> c-after-change-mark-abnormal-strings
> c-after-change-escape-NL-in-string))
> (advice-add name :override #'ignore '((name . joaot/remove-disable-some-cc-things)))
> (add-hook 'c-mode-common-hook
> (lambda ()
> (kill-local-variable 'electric-pair-inhibit-predicate)))))
Needless to say, my point of view with respect to the above is somewhat
different. Succinctly put, the minor modes electric-.... are MINOR
modes, and are quite new. They were implemented in a way which
interfered with major modes, and I can't see there was any need for
this. In particular, they interfered with CC Mode features which had
existed for 20 years at the time.
> On the C++/C editing-front, I am eagerly waiting for Treesitter to
> arrive so that is it becomes possible to write a new major mode for c++
> and c from scratch.
I am also looking forward to Treesitter, though I think starting a new
C++ Mode from scratch would not be a good use of time, and would be
needlessly disruptive for current CC Mode users.
> Reusing solid and efficient parsing logic based on language grammars.
> Another more closely available option, is to simply use LSP for
> indentation and fontification. Though that is almost certainly slower
> and less confortable/portable/etc... Anyway, whatever the solution,
> I'm quite confident that such a mode won't have these characteristics
> that cc-mode has when used with `electric-pair-mode`.
> If you're writing C, and not C++, my also try Stefan Monnier's aptly
> named sm-c-mode, installable via M-x package-install RET sm-c-mode.
> Works fine with electric-pair-mode, but seems to indent very oddly for
> some reason.
I would rather suggest using straight C Mode, which works and works
well. If there are incompatibilities between CC Mode and
electric-pair-mode, and it seems there are, let's get these fixed.
> Hope this helps. I'm sorry I can't offer any better solutions.
Could you please express your expert view? In Jim's opening post he
implies that:
(i) electric-pair-mode should be active inside comments.
(ii) In some circumstances at least, typing a " immediately in front
of another " causes two "s to be inserted. Are these circumstances
spelt out anywhere?
(iii) Typing a " inside a string causes two "s to be inserted, leaving
point between the two new adjacent strings formed. This feature is
not intended to work in C++ raw strings, or other similar multi-line
strings in other CC Mode modes.
Are these three features all intended in electric-pair-mode?
Thanks!
> João
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-12 18:05 ` Jim Porter
2021-09-15 22:17 ` Jim Porter
@ 2021-09-16 20:49 ` Alan Mackenzie
2021-09-16 21:36 ` Jim Porter
1 sibling, 1 reply; 24+ messages in thread
From: Alan Mackenzie @ 2021-09-16 20:49 UTC (permalink / raw)
To: Jim Porter; +Cc: acm, 50538
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).
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 19:05 ` Alan Mackenzie
@ 2021-09-16 20:51 ` João Távora
2021-09-17 18:12 ` Alan Mackenzie
0 siblings, 1 reply; 24+ messages in thread
From: João Távora @ 2021-09-16 20:51 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: Jim Porter, Lars Ingebrigtsen, 50538, Dmitry Gutov
Hello Alan,
[I apologize in advance for any spelling mistakes, my spell checker has
taken a vacation in this new operating system.]
Alan Mackenzie <acm@muc.de> writes:
> I think it is up to both of us to make this relationship less rocky.
Agreed. But I'm afraid I don't have much to add the discussion. I've
made all my points. And so have you. I understand, though I
respectfully disagree with your standpoint. And so do you, I hope. I
have, in a way, given up on this integration. electric-pair-mode
doesn't support cc-mode or any more directly derived from it
"officially". That harmony ended in Emacs 25.x, I believe (might have
been 26.x). After that, you may have some success with it, or you may
not.
>> (add-hook 'c-mode-common-hook
>> (lambda ()
>> (kill-local-variable 'electric-pair-inhibit-predicate)))))
>
> Needless to say, my point of view with respect to the above is somewhat
> different. Succinctly put, the minor modes electric-.... are MINOR
> modes, and are quite new.
> In particular, they interfered with CC Mode features which had existed
> for 20 years at the time.
My belief is that the term "minor" (which you so emphasize) in the name
"minor-mode" doesn't mean "less important" in any way. In my view, it
means merely "transversal". You've often pitched e-p-m and cc-mode
against each other in terms of their relative ages, implying a hierarchy
of importance, deference or prestige. My belief is that this is
unproductive. Emacs minor modes, old or new, are not less or more
important. All other things being equal, they don't deserve less or
more care and attention than Major or "old" modes. The only criteria to
motivate a change is technical.
> modes, and are quite new. They were implemented in a way which
> interfered with major modes,
You mention "major modes" in the plural form, but it is fair for me to
point out that only sub-modes of the cc-mode which you command entirely
are in that set. If I'm forgetting any others, you're welcome to
refresh my memory.
> and I can't see there was any need for this.
I hope you will first notice I didn't adjectify or make any value
judgement on the motivation, necessity or pertinence of your work in CC
mode. Therefore, I would appreciate if you could return the courtesy.
If you can't personally "see" the need for my work or how it was
performed, that's fine. I don't demand that you do, though I have gone
to some lengths in the past to inform you of the motivation. But that
isn't the same as there not being a such a need, or that I worked
spuriously, gratutiously or recklessly or with the intent to interfere
with anything. In fact, the Git log and users memory records clearly
that when e-p-m was first introduced in Emacs, it worked harmoniously
with cc-mode for a number of years until cc-mode was modified by you to
change that harmony.
Moreover, as far as I gather, electric-pair-mode is a fairly popular
minor mode, so at least some significant group of people adhere to the
same need that I saw. Even e-p-m's now abandoned predecessor,
"autopair"[1], is still fairly popular today. Before archiving it some
time ago, I was still seeing healthy download counts. See also [2] for
some Emacswiki user's (not me) account of the history and relationship
between autopair, e-p-m and other autopairing solutions for Emacs.
Now, to practical matters:
* If CC-mode users agree with you about the absence of a need for such a
thing as e-p-m, they may simply not turn it on (same goes for
electric-layout-mode and electric-foo-mode I suppose). I believe you
yourself and Eli are in this group.
* If CC-mode users disagree with you, they must find some alternative to
CC-mode that lets them confortably turn on those modes there. My two
self-described hacks are such alternatives. Not ideal ones, but
reasonably workable.
As a user, I chose the latter option. I look forward to a different
Major mode for editing C/CC code. That is my personal ambition as a
user.
As a developer, changing electric-pair-mode's core principles that work
harmoniously (if not downright flawlessly) for any other major mode not
based on cc-mode is not on the table.
>> On the C++/C editing-front, I am eagerly waiting for Treesitter to
>> arrive so that is it becomes possible to write a new major mode for c++
>> and c from scratch.
>
> I am also looking forward to Treesitter, though I think starting a new
> C++ Mode from scratch would not be a good use of time, and would be
> needlessly disruptive for current CC Mode users.
I certainly didn't want to imply it would be _your_ time. I believe
what other people do with the time they generously offer us is their
business. Certainly, I wouldn't dream of implying that the features
you're adding to cc-mode's ever-growing code base are not a good use of
your time. Furthermore, I think Emacs is fertile in alternative ways to
solve similar problems. I know others disagree, but I think this is a
quality.
> Could you please express your expert view? In Jim's opening post he
> implies that:
> (i) electric-pair-mode should be active inside comments.
> (ii) In some circumstances at least, typing a " immediately in front
> of another " causes two "s to be inserted. Are these circumstances
> spelt out anywhere?
> (iii) Typing a " inside a string causes two "s to be inserted, leaving
> point between the two new adjacent strings formed. This feature is
> not intended to work in C++ raw strings, or other similar multi-line
> strings in other CC Mode modes.
> Are these three features all intended in electric-pair-mode?
(i) Yes. Much as syntax tables and C-M-f , C-M-b, C-M-u,
etc. navigation work is comments, so does e-p-m.
(ii) As you well note, it depends on the circumstances.
The circumstances are well spelt out in the tests. I can name one
here.
If you have the buffer with contents
"hello"
And you type a double quote at buffer ends, you get
"hello"""
This example was taken from one of the tests of the group
'pair-some-quotes-skip-others', found in electric-tests.el, as I
think you know.
As I think you also know, the principle of
'electric-pair-preserve-balance' stipulates that
'electric-pair-mode' will only make 'self-insert-command' do
"suprising electric behaviour" -- insert two delimiters or skip one
-- if that action helps keep or improve the balance of said
delimiter in the buffer.
Therefore, if you find a circumstance where e-p-m inserts two
quotes and that happens to _worsen_ the balancing of the buffer
(i.e. creates an unterminated string somewhere where previously
there wasn't), then you have found a bug in e-p-m. In that case,
please describe it clearly as a bug report.
However, and very importantly, you must find this circumstance in
any major mode _other_ than cc-mode. Because -- as I've tried to
make clear -- "all bets are off in cc-mode", so to speak. So if
you are finding this misbehaviour in e-p-m _in conjunction with
cc-mode_, I wouldn't bother reporting it. We've established that
the solution that I would propose to you is very likely not going
to appeal to you.
(iii) Yes. That is the way that e-p-m always worked. By behaving so,
the balancing state of the buffer is preseved.
Some packages, (like smartparens [3], I believe) will insert a
single quote preceded by an escape character. That action also
keeps the balance.
Perhaps e-p-m could accomodate this behaviour from smartparens.
If some users would like it, I think it can be easily done.
Best regards,
João
[1]: https://github.com/joaotavora/autopair/
[2]: https://www.emacswiki.org/emacs/AutoPairs
[3]: https://github.com/Fuco1/smartparens
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 20:49 ` Alan Mackenzie
@ 2021-09-16 21:36 ` Jim Porter
2021-09-17 17:08 ` Alan Mackenzie
0 siblings, 1 reply; 24+ messages in thread
From: Jim Porter @ 2021-09-16 21:36 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: 50538
[-- Attachment #1: Type: text/plain, Size: 1983 bytes --]
On 9/16/2021 1:49 PM, Alan Mackenzie wrote:
> 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".
Thanks, I've incorporated all these changes into the attached patch. The
only difference between my patch and the version you provided was to
keep the `(search-backward "\"")' portion from my patch, since the code
needs to find the last double-quote, not the end of line.
As an aside, it's probably worth explaining why my patch searches for
the last double-quote in the first place. As far as I understand CC
Mode, when there's an unterminated double-quote on a line, both the
quote and the newline have a string fence property applied to them. This
means we could check the newline for that property, *but* there's no
guarantee a newline actually exists: `(c-point 'eoll)' could actually
point to the end of the buffer. To get around that, we search backwards
for the last double-quote of the (logical) line, since that's guaranteed
to exist.
If we wanted to, we could avoid searching backwards for the last
double-quote when a newline exists, but I'm not sure the gain in
performance (likely very small) is worth the extra code complexity.
[-- Attachment #2: 0001-Improve-behavior-of-electric-pair-mode-in-cc-mode.patch --]
[-- Type: text/plain, Size: 3293 bytes --]
From 4759ed0c8fa7fa8fafd47d166b3f126e606ffe84 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 16 Sep 2021 14:32:17 -0700
Subject: [PATCH] Improve behavior of 'electric-pair-mode' in 'cc-mode'
This ensures that quotes are paired correctly within comments, allows for
insertion of quote pairs immediately before another quote, and allows
inserting quote pairs within a string (thus splitting the string in two).
* lisp/progmodes/cc-mode.el (c-electric-pair-inhibit-predicate):
Inhibit insertion of paired quote in fewer cases.
* test/lisp/electric-tests.el (define-electric-pair-test):
Add 'c-mode' to list of modes to test by default.
---
lisp/progmodes/cc-mode.el | 22 +++++++++++++++-------
test/lisp/electric-tests.el | 5 ++++-
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 057d292246..60cea0b858 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -2550,18 +2550,26 @@ c-electric-pair-inhibit-predicate
At the time of call, point is just after the newly inserted CHAR.
-When CHAR is \", t will be returned unless the \" is marked with
-a string fence syntax-table text property. For other characters,
-the default value of `electric-pair-inhibit-predicate' is called
-and its value returned.
+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."
- (if (eq char ?\")
- (not (equal (get-text-property (1- (point)) 'c-fl-syn-tab) '(15)))
- (funcall (default-value 'electric-pair-inhibit-predicate) char)))
+ (or (and (eq char ?\")
+ (not (memq (cadr (c-semi-pp-to-literal (1- (point)))) '(c c++)))
+ (let ((last-quote (save-match-data
+ (save-excursion
+ (goto-char (c-point 'eoll))
+ (search-backward "\"")))))
+ (not (equal (c-get-char-property last-quote 'c-fl-syn-tab)
+ '(15)))))
+ (funcall (default-value 'electric-pair-inhibit-predicate) char)))
\f
;; Support for C
diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 666de89c53..35516a9f0b 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -32,6 +32,9 @@
(require 'elec-pair)
(require 'cl-lib)
+;; When running tests in c-mode, use single-line comments (//).
+(add-hook 'c-mode-hook (lambda () (c-toggle-comment-style -1)))
+
(defun call-with-saved-electric-modes (fn)
(let ((saved-electric (if electric-pair-mode 1 -1))
(saved-layout (if electric-layout-mode 1 -1))
@@ -173,7 +176,7 @@ define-electric-pair-test
expected-string
expected-point
bindings
- (modes '(quote (ruby-mode js-mode)))
+ (modes '(quote (ruby-mode js-mode c-mode)))
(test-in-comments t)
(test-in-strings t)
(test-in-code t)
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 21:36 ` Jim Porter
@ 2021-09-17 17:08 ` Alan Mackenzie
2021-09-23 2:01 ` bug#50538: [PATCH v3] " Jim Porter
0 siblings, 1 reply; 24+ messages in thread
From: Alan Mackenzie @ 2021-09-17 17:08 UTC (permalink / raw)
To: Jim Porter; +Cc: 50538
Hello, Jim.
On Thu, Sep 16, 2021 at 14:36:06 -0700, Jim Porter wrote:
> On 9/16/2021 1:49 PM, Alan Mackenzie wrote:
> > 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".
> Thanks, I've incorporated all these changes into the attached patch. The
> only difference between my patch and the version you provided was to
> keep the `(search-backward "\"")' portion from my patch, since the code
> needs to find the last double-quote, not the end of line.
Duh! Sorry about that, I clean forgot about it. I did say that I
hadn't tested it, though. ;-)
> As an aside, it's probably worth explaining why my patch searches for
> the last double-quote in the first place. As far as I understand CC
> Mode, when there's an unterminated double-quote on a line, both the
> quote and the newline have a string fence property applied to them.
Yes.
> This means we could check the newline for that property, *but* there's
> no guarantee a newline actually exists: `(c-point 'eoll)' could
> actually point to the end of the buffer. To get around that, we search
> backwards for the last double-quote of the (logical) line, since
> that's guaranteed to exist.
Yes. This is a very important case, the one that occurs when somebody's
typing a new file, or at the end of an existing one.
> If we wanted to, we could avoid searching backwards for the last
> double-quote when a newline exists, but I'm not sure the gain in
> performance (likely very small) is worth the extra code complexity.
I'm fairly sure it wouldn't be.
[ Patch snipped but read. ]
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-16 20:51 ` João Távora
@ 2021-09-17 18:12 ` Alan Mackenzie
0 siblings, 0 replies; 24+ messages in thread
From: Alan Mackenzie @ 2021-09-17 18:12 UTC (permalink / raw)
To: João Távora
Cc: Jim Porter, 50538, Dmitry Gutov, acm, Lars Ingebrigtsen
Hello, João.
On Thu, Sep 16, 2021 at 21:51:13 +0100, João Távora wrote:
> Hello Alan,
> [I apologize in advance for any spelling mistakes, my spell checker has
> taken a vacation in this new operating system.]
> Alan Mackenzie <acm@muc.de> writes:
> > I think it is up to both of us to make this relationship less rocky.
> Agreed. But I'm afraid I don't have much to add the discussion. I've
> made all my points. And so have you.
I was thinking more along the lines of coding, not discussion.
> I understand, though I respectfully disagree with your standpoint.
> And so do you, I hope. I have, in a way, given up on this
> integration. electric-pair-mode doesn't support cc-mode or any more
> directly derived from it "officially". That harmony ended in Emacs
> 25.x, I believe (might have been 26.x). After that, you may have some
> success with it, or you may not.
Why such a negative attitude? CC Mode is an important package, and so
is electric-pair-mode. There doesn't seem to be any intrinsic reason
why they shouldn't work together better.
> >> (add-hook 'c-mode-common-hook
> >> (lambda ()
> >> (kill-local-variable 'electric-pair-inhibit-predicate)))))
> > Needless to say, my point of view with respect to the above is somewhat
> > different. Succinctly put, the minor modes electric-.... are MINOR
> > modes, and are quite new.
> > In particular, they interfered with CC Mode features which had existed
> > for 20 years at the time.
> My belief is that the term "minor" (which you so emphasize) in the name
> "minor-mode" doesn't mean "less important" in any way.
No, that's not how I meant it. It's more that the layout of the text on
the screen, and the process of changing it ARE the major mode. Any
interference with the major mode in its primary role is a buggy
situation needing fixing.
> In my view, it means merely "transversal". You've often pitched e-p-m
> and cc-mode against each other in terms of their relative ages,
> implying a hierarchy of importance, deference or prestige.
More a regret that when the electric-... modes were developed, they paid
no attention to CC Mode, even when the feature originated there. This
was bound to lead to conflict sooner or later, and that is where we are
at now.
> My belief is that this is unproductive. Emacs minor modes, old or
> new, are not less or more important. All other things being equal,
> they don't deserve less or more care and attention than Major or "old"
> modes. The only criteria to motivate a change is technical.
Right now, CC Mode binds post-self-insert-hook to nil when calling
self-insert-function, so as to get it functioning predictably, then
attempts to compensate later by calling e.g.
electric-pair-post-self-insert-function directly. I think you'll agree
with me this is suboptimal. I can't see any way of getting around it
without amendments to electric-pair-mode.
> > modes, and are quite new. They were implemented in a way which
> > interfered with major modes,
[ .... ]
> > and I can't see there was any need for this.
> I hope you will first notice I didn't adjectify or make any value
> judgement on the motivation, necessity or pertinence of your work in CC
> mode.
You seem to me to be doing exactly this by declining to work to improve
the interface between CC Mode and e-p-m.
> Therefore, I would appreciate if you could return the courtesy. If
> you can't personally "see" the need for my work or how it was
> performed, that's fine.
I can. I can also see problems with it, and regret your very negative
attitude towards fixing problems in your own area of speciality.
CC Mode has existed in one form or another for around 40 years. I think
it would be reasonable for a new minor mode to take due account of it
when being developed. As far as I can see, this didn't happen with
electric-pair-mode to a sufficient degree.
> I don't demand that you do, though I have gone to some lengths in the
> past to inform you of the motivation. But that isn't the same as
> there not being a such a need, or that I worked spuriously,
> gratutiously or recklessly or with the intent to interfere with
> anything.
I am not implying such, and don't believe I ever have done.
> In fact, the Git log and users memory records clearly that when e-p-m
> was first introduced in Emacs, it worked harmoniously with cc-mode for
> a number of years until cc-mode was modified by you to change that
> harmony.
The use of post-self-insert-hook to make buffer changes has never worked
well with CC Mode. It cannot work in any major mode which uses
self-insert-command as part of a mode command.
You seem to be implying that my motivation in developing CC Mode was to
damage the workings of e-p-m. I think that's somewhat uncalled for. CC
Mode is under continual development, and has traditionally been at the
forefront of new techniques. Electric indentation, subword mode, and
the correct fontification of unterminated strings are just three
examples of this.
When you drew attention to problems, I put considerable effort into
fixing and ameliorating these in CC Mode.
> Moreover, as far as I gather, electric-pair-mode is a fairly popular
> minor mode, so at least some significant group of people adhere to the
> same need that I saw. Even e-p-m's now abandoned predecessor,
> "autopair"[1], is still fairly popular today. Before archiving it some
> time ago, I was still seeing healthy download counts. See also [2] for
> some Emacswiki user's (not me) account of the history and relationship
> between autopair, e-p-m and other autopairing solutions for Emacs.
Yes, CC Mode and e-p-m are both popular and important. They don't work
well enough together.
> Now, to practical matters:
> * If CC-mode users agree with you about the absence of a need for such a
> thing as e-p-m, they may simply not turn it on (same goes for
> electric-layout-mode and electric-foo-mode I suppose). I believe you
> yourself and Eli are in this group.
I'm not in such a group. I accept the need for e-p-m. I just don't
want to use it myself.
> * If CC-mode users disagree with you, they must find some alternative to
> CC-mode that lets them confortably turn on those modes there. My two
> self-described hacks are such alternatives. Not ideal ones, but
> reasonably workable.
Why don't you put some work in to make e-p-m work better with CC Mode?
Maybe I'm misremembering here, but when the problems raised themselves I
don't recall you making, or being prepared to make any amendments to
electric-pair-mode. Why not?
> As a user, I chose the latter option. I look forward to a different
> Major mode for editing C/CC code. That is my personal ambition as a
> user.
You might have a long wait. And people who run from things because they
don't like them often don't like what they find instead, either.
Are you saying that your aim is to _prevent_ e-p-m working well with CC
Mode?
> As a developer, changing electric-pair-mode's core principles that work
> harmoniously (if not downright flawlessly) for any other major mode not
> based on cc-mode is not on the table.
That's a bit callous, isn't it? CC Mode is a highly important part of
Emacs, and you're deliberately not catering to it? Why?
Why not amend e-p-m so that it will work harmoniously with _any_ major
mode, not just the simple ones? Where's the difficulty?
[ .... ]
[ Details of particulars of electric-pair-mode operation read with
thanks. ]
> Best regards,
> João
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH v3] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-17 17:08 ` Alan Mackenzie
@ 2021-09-23 2:01 ` Jim Porter
2021-09-26 20:58 ` Alan Mackenzie
0 siblings, 1 reply; 24+ messages in thread
From: Jim Porter @ 2021-09-23 2:01 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: 50538
[-- Attachment #1: Type: text/plain, Size: 207 bytes --]
Ok, I've attached an updated patch that should apply cleanly on top of
the patches for bug#49518. The tests still pass locally for me. If
there's anything else that needs fixed/adjusted, just let me know.
[-- Attachment #2: 0001-Improve-behavior-of-electric-pair-mode-in-cc-mode.patch --]
[-- Type: text/plain, Size: 3317 bytes --]
From c4d069c010d5e526fa85d5800ff8de5bb6a52b7e Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 16 Sep 2021 14:32:17 -0700
Subject: [PATCH] Improve behavior of 'electric-pair-mode' in 'cc-mode'
This ensures that quotes are paired correctly within comments, allows for
insertion of quote pairs immediately before another quote, and allows
inserting quote pairs within a string (thus splitting the string in two).
* lisp/progmodes/cc-mode.el (c-electric-pair-inhibit-predicate):
Inhibit insertion of paired quote in fewer cases.
* test/lisp/electric-tests.el (define-electric-pair-test):
Add 'c-mode' to list of modes to test by default.
---
lisp/progmodes/cc-mode.el | 22 +++++++++++++++-------
test/lisp/electric-tests.el | 5 ++++-
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 8b30241449..80137590c7 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -2549,18 +2549,26 @@ c-electric-pair-inhibit-predicate
At the time of call, point is just after the newly inserted CHAR.
-When CHAR is \", t will be returned unless the \" is marked with
-a string fence syntax-table text property. For other characters,
-the default value of `electric-pair-inhibit-predicate' is called
-and its value returned.
+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."
- (if (eq char ?\")
- (not (equal (get-text-property (1- (point)) 'c-fl-syn-tab) '(15)))
- (funcall (default-value 'electric-pair-inhibit-predicate) char)))
+ (or (and (eq char ?\")
+ (not (memq (cadr (c-semi-pp-to-literal (1- (point)))) '(c c++)))
+ (let ((last-quote (save-match-data
+ (save-excursion
+ (goto-char (c-point 'eoll))
+ (search-backward "\"")))))
+ (not (equal (c-get-char-property last-quote 'c-fl-syn-tab)
+ '(15)))))
+ (funcall (default-value 'electric-pair-inhibit-predicate) char)))
\f
;; Support for C
diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 46bcbfce30..4b3a39b85f 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -32,6 +32,9 @@
(require 'elec-pair)
(require 'cl-lib)
+;; When running tests in c-mode, use single-line comments (//).
+(add-hook 'c-mode-hook (lambda () (c-toggle-comment-style -1)))
+
(defun call-with-saved-electric-modes (fn)
(let ((saved-electric (if electric-pair-mode 1 -1))
(saved-layout (if electric-layout-mode 1 -1))
@@ -174,7 +177,7 @@ define-electric-pair-test
expected-string
expected-point
bindings
- (modes '(quote (ruby-mode js-mode python-mode)))
+ (modes '(quote (ruby-mode js-mode python-mode c-mode)))
(test-in-comments t)
(test-in-strings t)
(test-in-code t)
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#50538: [PATCH v3] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-23 2:01 ` bug#50538: [PATCH v3] " Jim Porter
@ 2021-09-26 20:58 ` Alan Mackenzie
2021-09-28 4:57 ` bug#50538: [PATCH v4] " Jim Porter
0 siblings, 1 reply; 24+ messages in thread
From: Alan Mackenzie @ 2021-09-26 20:58 UTC (permalink / raw)
To: Jim Porter; +Cc: 50538
Hello, Jim.
There's one thing I'm a bit uncertain about in the patch for cc-mode.el.
On Wed, Sep 22, 2021 at 19:01:11 -0700, Jim Porter wrote:
> Ok, I've attached an updated patch that should apply cleanly on top of
> the patches for bug#49518. The tests still pass locally for me. If
> there's anything else that needs fixed/adjusted, just let me know.
[ .... ]
> diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
> index 8b30241449..80137590c7 100644
> --- a/lisp/progmodes/cc-mode.el
> +++ b/lisp/progmodes/cc-mode.el
> @@ -2549,18 +2549,26 @@ c-electric-pair-inhibit-predicate
> At the time of call, point is just after the newly inserted CHAR.
> -When CHAR is \", t will be returned unless the \" is marked with
> -a string fence syntax-table text property. For other characters,
> -the default value of `electric-pair-inhibit-predicate' is called
> -and its value returned.
> +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."
> - (if (eq char ?\")
> - (not (equal (get-text-property (1- (point)) 'c-fl-syn-tab) '(15)))
> - (funcall (default-value 'electric-pair-inhibit-predicate) char)))
> + (or (and (eq char ?\")
> + (not (memq (cadr (c-semi-pp-to-literal (1- (point)))) '(c c++)))
> + (let ((last-quote (save-match-data
> + (save-excursion
> + (goto-char (c-point 'eoll))
> + (search-backward "\"")))))
> + (not (equal (c-get-char-property last-quote 'c-fl-syn-tab)
> + '(15)))))
> + (funcall (default-value 'electric-pair-inhibit-predicate) char)))
In the line starting (or (and (eq char ?\"), don't we still need an `if'
form? I think that otherwise, if any of the sub-forms of the `and'
return nil, we will call (default-value
'electric-pair-inhibit-predicate), which surely isn't what we want. If
we have a ", we want the CC Mode function to do all the work, only
delegating to the standard function when we don't have a ".
Or have I missed something?
[ .... ]
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 24+ messages in thread
* bug#50538: [PATCH v4] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-26 20:58 ` Alan Mackenzie
@ 2021-09-28 4:57 ` Jim Porter
2021-10-03 17:58 ` bug#50538: [PATCH v5] " Jim Porter
0 siblings, 1 reply; 24+ messages in thread
From: Jim Porter @ 2021-09-28 4:57 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: 50538
[-- Attachment #1: Type: text/plain, Size: 2663 bytes --]
On 9/26/2021 1:58 PM, Alan Mackenzie wrote:
>
> There's one thing I'm a bit uncertain about in the patch for cc-mode.el.
>
> On Wed, Sep 22, 2021 at 19:01:11 -0700, Jim Porter wrote:
[snip]
>> - (if (eq char ?\")
>> - (not (equal (get-text-property (1- (point)) 'c-fl-syn-tab) '(15)))
>> - (funcall (default-value 'electric-pair-inhibit-predicate) char)))
>> + (or (and (eq char ?\")
>> + (not (memq (cadr (c-semi-pp-to-literal (1- (point)))) '(c c++)))
>> + (let ((last-quote (save-match-data
>> + (save-excursion
>> + (goto-char (c-point 'eoll))
>> + (search-backward "\"")))))
>> + (not (equal (c-get-char-property last-quote 'c-fl-syn-tab)
>> + '(15)))))
>> + (funcall (default-value 'electric-pair-inhibit-predicate) char)))
>
> In the line starting (or (and (eq char ?\"), don't we still need an `if'
> form? I think that otherwise, if any of the sub-forms of the `and'
> return nil, we will call (default-value
> 'electric-pair-inhibit-predicate), which surely isn't what we want. If
> we have a ", we want the CC Mode function to do all the work, only
> delegating to the standard function when we don't have a ".
>
> Or have I missed something?
I don't have a strong opinion on this either way, so here's a patch that
uses an `if' form as you suggest. However, I introduced the `or' form
in response to Eli's concern[1]:
On 9/11/2021 11:26 PM, Eli Zaretskii wrote:
> 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? It could be that the user wants to modify the existing string
> instead, in which case your suggested patches will require the user
> to delete more quotes than previously.
I discussed the pros and cons in my followup[2]:
> 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 [override `c-electric-pair-inhibit-predicate'].[3]
Personally, I'm fine with letting CC Mode do all the work here, and
requiring users to advise this function if they want a different
behavior. It's probably easier to be sure that things don't break by
using the `if' form patch, though I think the previous `or' form patch
should be pretty robust too.
[1] https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-09/msg00976.html
[2] https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-09/msg01014.html
[3] My explanation in the original message was incorrect, so I've fixed
it here.
[-- Attachment #2: 0001-Improve-behavior-of-electric-pair-mode-in-cc-mode.patch --]
[-- Type: text/plain, Size: 3317 bytes --]
From 5c12fe09f7ac710b2c2f279e0faa04be8652db0f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 16 Sep 2021 14:32:17 -0700
Subject: [PATCH] Improve behavior of 'electric-pair-mode' in 'cc-mode'
This ensures that quotes are paired correctly within comments, allows for
insertion of quote pairs immediately before another quote, and allows
inserting quote pairs within a string (thus splitting the string in two).
* lisp/progmodes/cc-mode.el (c-electric-pair-inhibit-predicate):
Inhibit insertion of paired quote in fewer cases.
* test/lisp/electric-tests.el (define-electric-pair-test):
Add 'c-mode' to list of modes to test by default.
---
lisp/progmodes/cc-mode.el | 22 +++++++++++++++-------
test/lisp/electric-tests.el | 5 ++++-
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 8b30241449..80137590c7 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -2549,18 +2549,26 @@ c-electric-pair-inhibit-predicate
At the time of call, point is just after the newly inserted CHAR.
-When CHAR is \", t will be returned unless the \" is marked with
-a string fence syntax-table text property. For other characters,
-the default value of `electric-pair-inhibit-predicate' is called
-and its value returned.
+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."
- (if (eq char ?\")
- (not (equal (get-text-property (1- (point)) 'c-fl-syn-tab) '(15)))
- (funcall (default-value 'electric-pair-inhibit-predicate) char)))
+ (or (and (eq char ?\")
+ (not (memq (cadr (c-semi-pp-to-literal (1- (point)))) '(c c++)))
+ (let ((last-quote (save-match-data
+ (save-excursion
+ (goto-char (c-point 'eoll))
+ (search-backward "\"")))))
+ (not (equal (c-get-char-property last-quote 'c-fl-syn-tab)
+ '(15)))))
+ (funcall (default-value 'electric-pair-inhibit-predicate) char)))
\f
;; Support for C
diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 4e7cbf5419..5eec058ede 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -32,6 +32,9 @@
(require 'elec-pair)
(require 'cl-lib)
+;; When running tests in c-mode, use single-line comments (//).
+(add-hook 'c-mode-hook (lambda () (c-toggle-comment-style -1)))
+
(defun call-with-saved-electric-modes (fn)
(let ((saved-electric (if electric-pair-mode 1 -1))
(saved-layout (if electric-layout-mode 1 -1))
@@ -174,7 +177,7 @@ define-electric-pair-test
expected-string
expected-point
bindings
- (modes '(quote (ruby-mode js-mode python-mode)))
+ (modes '(quote (ruby-mode js-mode python-mode c-mode)))
(test-in-comments t)
(test-in-strings t)
(test-in-code t)
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#50538: [PATCH v5] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-09-28 4:57 ` bug#50538: [PATCH v4] " Jim Porter
@ 2021-10-03 17:58 ` Jim Porter
2021-11-06 0:22 ` bug#50538: [PATCH] " Lars Ingebrigtsen
0 siblings, 1 reply; 24+ messages in thread
From: Jim Porter @ 2021-10-03 17:58 UTC (permalink / raw)
To: Alan Mackenzie; +Cc: 50538
[-- Attachment #1: Type: text/plain, Size: 466 bytes --]
On 9/27/2021 9:57 PM, Jim Porter wrote:
> I don't have a strong opinion on this either way, so here's a patch that
> uses an `if' form as you suggest.
Oops, I was looking over this patch again and realized that I simply
re-attached the old patch. Sorry about that. Here's the version that
*really* uses the `if' form. Like I said in my previous message, I don't
have a strong opinion on whether to use the `if' form or the `or' form;
either works fine for me.
[-- Attachment #2: 0001-Improve-behavior-of-electric-pair-mode-in-cc-mode.patch --]
[-- Type: text/plain, Size: 3213 bytes --]
From 99d2e7efa08e906a21b05128e02e8ad967219d0b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 3 Oct 2021 10:46:01 -0700
Subject: [PATCH] Improve behavior of 'electric-pair-mode' in 'cc-mode'
This ensures that quotes are paired correctly within comments, allows for
insertion of quote pairs immediately before another quote, and allows
inserting quote pairs within a string (thus splitting the string in two).
* lisp/progmodes/cc-mode.el (c-electric-pair-inhibit-predicate):
Inhibit insertion of paired quote in fewer cases.
* test/lisp/electric-tests.el (define-electric-pair-test):
Add 'c-mode' to list of modes to test by default.
---
lisp/progmodes/cc-mode.el | 19 +++++++++++++------
test/lisp/electric-tests.el | 5 ++++-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 8b30241449..2d7fb731e6 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -2549,17 +2549,24 @@ c-electric-pair-inhibit-predicate
At the time of call, point is just after the newly inserted CHAR.
-When CHAR is \", t will be returned unless the \" is marked with
-a string fence syntax-table text property. For other characters,
-the default value of `electric-pair-inhibit-predicate' is called
-and its value returned.
+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."
- (if (eq char ?\")
- (not (equal (get-text-property (1- (point)) 'c-fl-syn-tab) '(15)))
+ (if (and (eq char ?\")
+ (not (memq (cadr (c-semi-pp-to-literal (1- (point)))) '(c c++))))
+ (let ((last-quote (save-match-data
+ (save-excursion
+ (goto-char (c-point 'eoll))
+ (search-backward "\"")))))
+ (not (equal (c-get-char-property last-quote 'c-fl-syn-tab) '(15))))
(funcall (default-value 'electric-pair-inhibit-predicate) char)))
\f
diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el
index 4e7cbf5419..5eec058ede 100644
--- a/test/lisp/electric-tests.el
+++ b/test/lisp/electric-tests.el
@@ -32,6 +32,9 @@
(require 'elec-pair)
(require 'cl-lib)
+;; When running tests in c-mode, use single-line comments (//).
+(add-hook 'c-mode-hook (lambda () (c-toggle-comment-style -1)))
+
(defun call-with-saved-electric-modes (fn)
(let ((saved-electric (if electric-pair-mode 1 -1))
(saved-layout (if electric-layout-mode 1 -1))
@@ -174,7 +177,7 @@ define-electric-pair-test
expected-string
expected-point
bindings
- (modes '(quote (ruby-mode js-mode python-mode)))
+ (modes '(quote (ruby-mode js-mode python-mode c-mode)))
(test-in-comments t)
(test-in-strings t)
(test-in-code t)
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode
2021-10-03 17:58 ` bug#50538: [PATCH v5] " Jim Porter
@ 2021-11-06 0:22 ` Lars Ingebrigtsen
0 siblings, 0 replies; 24+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-06 0:22 UTC (permalink / raw)
To: Jim Porter; +Cc: Alan Mackenzie, 50538
Jim Porter <jporterbugs@gmail.com> writes:
> Oops, I was looking over this patch again and realized that I simply
> re-attached the old patch. Sorry about that. Here's the version that
> *really* uses the `if' form. Like I said in my previous message, I
> don't have a strong opinion on whether to use the `if' form or the
> `or' form; either works fine for me.
Skimming this thread, it seems that everybody was OK with this patch,
but it wasn't applied. So I've now done so (but in Emacs 29).
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-11-06 0:22 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-12 3:58 bug#50538: [PATCH] 28.0.50; electric-pair-mode fails to pair double quotes in some cases in CC mode Jim Porter
2021-09-12 6:26 ` Eli Zaretskii
2021-09-12 18:05 ` Jim Porter
2021-09-15 22:17 ` Jim Porter
2021-09-16 5:25 ` Eli Zaretskii
2021-09-16 12:40 ` Lars Ingebrigtsen
2021-09-16 12:59 ` Dmitry Gutov
2021-09-16 13:17 ` Lars Ingebrigtsen
2021-09-16 17:04 ` João Távora
2021-09-16 17:11 ` Eli Zaretskii
2021-09-16 17:33 ` João Távora
2021-09-16 17:29 ` Jim Porter
2021-09-16 19:05 ` Alan Mackenzie
2021-09-16 20:51 ` João Távora
2021-09-17 18:12 ` Alan Mackenzie
2021-09-16 18:26 ` Alan Mackenzie
2021-09-16 20:49 ` Alan Mackenzie
2021-09-16 21:36 ` Jim Porter
2021-09-17 17:08 ` Alan Mackenzie
2021-09-23 2:01 ` bug#50538: [PATCH v3] " Jim Porter
2021-09-26 20:58 ` Alan Mackenzie
2021-09-28 4:57 ` bug#50538: [PATCH v4] " Jim Porter
2021-10-03 17:58 ` bug#50538: [PATCH v5] " Jim Porter
2021-11-06 0:22 ` bug#50538: [PATCH] " Lars Ingebrigtsen
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).