* 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: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 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: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-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: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] 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-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 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 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).