Reported by: bea <at> klebe.blog
Date: Tue, 18 Dec 2018 17:48:02 UTC
Severity: normal
Found in version 26.1
Done: Alan Mackenzie <acm <at> muc.de>
Bug is archived. No further changes may be made.
Message #47 received at 33794 <at> debbugs.gnu.org (full text, mbox):
From: João Távora <joaotavora <at> gmail.com> To: Beatrix Klebe <beeuhtricks <at> gmail.com> Cc: Alan Mackenzie <acm <at> muc.de>, bea <at> klebe.blog, Stefan Monnier <monnier <at> iro.umontreal.ca>, 33794 <at> debbugs.gnu.org Subject: Re: bug#33794: 26.1; electric-pair-mode breaks auto-newline minor mode of cc-mode Date: Sat, 22 Dec 2018 02:16:03 +0000
[Message part 1 (text/plain, inline)]
Hi again Beatrix, In the latest version of the patch I am preparing, if you do (add-hook 'c-mode-hook (lambda () (setq-local electric-layout-rules '((?\{ . before}) (?\{ . after) (?\{ . after-stay))))) You should arive at Allman C-style electric layouts. So typing int main () <spaces> then brace should give you: int main () { <cursor here> } I attach the two patches for convenience, which you can also find in the scratch/fix-33794-extend-electric-layout-mode branch. João
[0001-Extend-electric-layout-mode-to-handle-more-complex-l.patch (text/x-diff, inline)]
From ab036bdedbb49ecc96d550b5e883e43bb03eaccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com> Date: Fri, 21 Dec 2018 18:00:08 +0000 Subject: [PATCH 1/2] Extend electric-layout-mode to handle more complex layouts Also, have it play nice with electric-pair-mode. Multiple matching entries in `electric-layout-rules' are executed in order of appearance. When inserting a newline in the 'after-stay rule, ensure electric-pair-open-newline-between-pairs is nil. Arguably the logic behind electric-pair-open-newline-between-pairs should be moved to electric-layout-mode, but the current rule-matching engine doesn't allow for it. The current solution seems to be good enough for the situations reported in bug#33794. * lisp/electric.el (electric-layout-rules): Adjust docstring. (electric-layout-post-self-insert-function): Loop through rules. Bind electric-pair-open-newline-between-pairs to nil when handling after-stay. --- lisp/electric.el | 61 ++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/lisp/electric.el b/lisp/electric.el index 6dbf46b80c..6a307a49b9 100644 --- a/lisp/electric.el +++ b/lisp/electric.el @@ -370,38 +370,43 @@ electric-layout-rules The symbols specify where in relation to CHAR the newline character(s) should be inserted. `after-stay' means insert a -newline after CHAR but stay in the same place.") +newline after CHAR but stay in the same place. + +If multiple rules match, they are all executed in order of +appearance.") (defun electric-layout-post-self-insert-function () - (let* ((rule (cdr (assq last-command-event electric-layout-rules))) - pos) - (when (and rule - (setq pos (electric--after-char-pos)) + (let (pos) + (when (and (setq pos (electric--after-char-pos)) ;; Not in a string or comment. (not (nth 8 (save-excursion (syntax-ppss pos))))) - (let ((end (point-marker)) - (sym (if (functionp rule) (funcall rule) rule))) - (set-marker-insertion-type end (not (eq sym 'after-stay))) - (goto-char pos) - (pcase sym - ;; FIXME: we used `newline' down here which called - ;; self-insert-command and ran post-self-insert-hook recursively. - ;; It happened to make electric-indent-mode work automatically with - ;; electric-layout-mode (at the cost of re-indenting lines - ;; multiple times), but I'm not sure it's what we want. - ;; - ;; FIXME: check eolp before inserting \n? - ('before (goto-char (1- pos)) (skip-chars-backward " \t") - (unless (bolp) (insert "\n"))) - ('after (insert "\n")) - ('after-stay (save-excursion - (let ((electric-layout-rules nil)) - (newline 1 t)))) - ('around (save-excursion - (goto-char (1- pos)) (skip-chars-backward " \t") - (unless (bolp) (insert "\n"))) - (insert "\n"))) ; FIXME: check eolp before inserting \n? - (goto-char end))))) + (goto-char pos) + (dolist (rule electric-layout-rules) + (when (eq last-command-event (car rule)) + (let* ((end (point-marker)) + (rule (cdr rule)) + (sym (if (functionp rule) (funcall rule) rule))) + (set-marker-insertion-type end (not (eq sym 'after-stay))) + (pcase sym + ;; FIXME: we used `newline' down here which called + ;; self-insert-command and ran post-self-insert-hook recursively. + ;; It happened to make electric-indent-mode work automatically with + ;; electric-layout-mode (at the cost of re-indenting lines + ;; multiple times), but I'm not sure it's what we want. + ;; + ;; FIXME: check eolp before inserting \n? + ('before (goto-char (1- pos)) (skip-chars-backward " \t") + (unless (bolp) (insert "\n"))) + ('after (insert "\n")) + ('after-stay (save-excursion + (let ((electric-layout-rules nil) + (electric-pair-open-newline-between-pairs nil)) + (newline 1 t)))) + ('around (save-excursion + (goto-char (1- pos)) (skip-chars-backward " \t") + (unless (bolp) (insert "\n"))) + (insert "\n"))) ; FIXME: check eolp before inserting \n? + (goto-char end))))))) (put 'electric-layout-post-self-insert-function 'priority 40) -- 2.20.0
[0002-Rework-electric-layout-post-self-insert-function-bug.patch (text/x-diff, inline)]
From dfc19a04f5f455051d158648a963cb1fd8b91c67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com> Date: Sat, 22 Dec 2018 01:52:08 +0000 Subject: [PATCH 2/2] Rework electric-layout-post-self-insert-function (bug#33794) This should now fix more problems reported in (bug#33794) regarding insertion of newlines before and after the opening brace. Write two automated tests. Also provide a new electric-layout-local-mode for testing. * lisp/electric.el (electric-layout-post-self-insert-function-1): New function that does the work for electric-layout-post-self-insert-function-1. (electric-layout-local-mode): New minor mode. * test/lisp/electric-tests.el (electric-layout-int-main-kernel-style) (electric-layout-int-main-allman-style): Add two tests. --- lisp/electric.el | 49 +++++++++++++++++++++++++------------ test/lisp/electric-tests.el | 34 +++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/lisp/electric.el b/lisp/electric.el index 6a307a49b9..24c040d05b 100644 --- a/lisp/electric.el +++ b/lisp/electric.el @@ -376,17 +376,25 @@ electric-layout-rules appearance.") (defun electric-layout-post-self-insert-function () - (let (pos) + (when electric-layout-mode + (electric-layout-post-self-insert-function-1))) + +;; for edebug's sake a separate function +(defun electric-layout-post-self-insert-function-1 () + (let (pos end) (when (and (setq pos (electric--after-char-pos)) ;; Not in a string or comment. (not (nth 8 (save-excursion (syntax-ppss pos))))) (goto-char pos) + (setq end (point-marker)) (dolist (rule electric-layout-rules) (when (eq last-command-event (car rule)) - (let* ((end (point-marker)) - (rule (cdr rule)) - (sym (if (functionp rule) (funcall rule) rule))) - (set-marker-insertion-type end (not (eq sym 'after-stay))) + (let* ((rule (cdr rule)) + (sym (if (functionp rule) (funcall rule) rule)) + (nl (lambda () + (let ((electric-layout-mode nil) + (electric-pair-open-newline-between-pairs nil)) + (newline 1 t))))) (pcase sym ;; FIXME: we used `newline' down here which called ;; self-insert-command and ran post-self-insert-hook recursively. @@ -395,18 +403,16 @@ electric-layout-post-self-insert-function ;; multiple times), but I'm not sure it's what we want. ;; ;; FIXME: check eolp before inserting \n? - ('before (goto-char (1- pos)) (skip-chars-backward " \t") - (unless (bolp) (insert "\n"))) - ('after (insert "\n")) - ('after-stay (save-excursion - (let ((electric-layout-rules nil) - (electric-pair-open-newline-between-pairs nil)) - (newline 1 t)))) + ('before (save-excursion + (goto-char (1- pos)) (skip-chars-backward " \t") + (unless (bolp) (funcall nl)))) + ('after (funcall nl)) + ('after-stay (save-excursion (funcall nl))) ('around (save-excursion (goto-char (1- pos)) (skip-chars-backward " \t") - (unless (bolp) (insert "\n"))) - (insert "\n"))) ; FIXME: check eolp before inserting \n? - (goto-char end))))))) + (unless (bolp) (funcall nl))) + (funcall nl))) ; FIXME: check eolp before inserting \n? + )))))) (put 'electric-layout-post-self-insert-function 'priority 40) @@ -424,6 +430,19 @@ electric-layout-mode (remove-hook 'post-self-insert-hook #'electric-layout-post-self-insert-function)))) +;;;###autoload +(define-minor-mode electric-layout-local-mode + "Toggle `electric-layout-mode' only in this buffer." + :variable (buffer-local-value 'electric-layout-mode (current-buffer)) + (cond + ((eq electric-layout-mode (default-value 'electric-layout-mode)) + (kill-local-variable 'electric-layout-mode)) + ((not (default-value 'electric-layout-mode)) + ;; Locally enabled, but globally disabled. + (electric-layout-mode 1) ; Setup the hooks. + (setq-default electric-layout-mode nil) ; But keep it globally disabled. + ))) + ;;; Electric quoting. (defcustom electric-quote-comment t diff --git a/test/lisp/electric-tests.el b/test/lisp/electric-tests.el index a665d2eb28..971c3e9567 100644 --- a/test/lisp/electric-tests.el +++ b/test/lisp/electric-tests.el @@ -812,5 +812,39 @@ electric-quote-markdown-in-code :bindings '((comment-start . "<!--") (comment-use-syntax . t)) :test-in-comments nil :test-in-strings nil) + +;;; tests for `electric-layout-mode' + +(ert-deftest electric-layout-int-main-kernel-style () + (save-electric-modes + (ert-with-test-buffer () + (c-mode) + (electric-layout-local-mode 1) + (electric-pair-local-mode 1) + (electric-indent-local-mode 1) + (setq-local electric-layout-rules + '((?\{ . after) + (?\{ . after-stay))) + (insert "int main () ") + (let ((last-command-event ?\{)) + (call-interactively (key-binding `[,last-command-event]))) + (should (equal (buffer-string) "int main () {\n \n}"))))) + +(ert-deftest electric-layout-int-main-allman-style () + (save-electric-modes + (ert-with-test-buffer () + (c-mode) + (electric-layout-local-mode 1) + (electric-pair-local-mode 1) + (electric-indent-local-mode 1) + (setq-local electric-layout-rules + '((?\{ . before) + (?\{ . after) + (?\{ . after-stay))) + (insert "int main () ") + (let ((last-command-event ?\{)) + (call-interactively (key-binding `[,last-command-event]))) + (should (equal (buffer-string) "int main ()\n{\n \n}"))))) + (provide 'electric-tests) ;;; electric-tests.el ends here -- 2.20.0
[Message part 4 (text/plain, inline)]
João Távora <joaotavora <at> gmail.com> writes: > Beatrix Klebe <beeuhtricks <at> gmail.com> writes: > >> I know how to do hotpaches but that doesn't appear to solve the >> problem I'm having here, unless I've missed something. The problem is >> with moving the opening bracket, not the insertion point. > > Beatrix, > > I think you may have missed the fact that I am suggesting alternatives > that: > > * involve cc-mode, or one of its derived modes; > > * don't involve M-x c-toggle-auto-newline (turning on what you call > auto-newline-mode); > > * involve turning on the global electric-layout-mode and a thin > customization for it in the buffers where you think it's relevant > (presumably cc-mode); > > * may involve multiple fixed/patched versions of lisp/electric.el as I > understand your problem(s); > > As it stands, the last patch I sent you passes my only test which is > this: given a file 33794.el which is just: > > (electric-pair-mode) > (electric-layout-mode) > > (add-hook 'c-mode-hook > (lambda () > (setq-local electric-layout-rules > '((?\{ . after) > (?\{ . after-stay))))) > > > then running this from a shell: > > $ emacs -Q -l 33794.el something.c > > Opens a new c-mode buffer. Type 'int main ()' and then an opening > brace. You should get: > > int main () { > <cursor> > } > > Can you reproduce these results? If you can come up with more of these > tests written in this or a similarly simple and exact manner it's easier > for me to understand what's going on (it's also easier to write > automated tests). > > João > >> >> >> On Fri, Dec 21, 2018 at 2:20 PM João Távora <joaotavora <at> gmail.com> wrote: >>> >>> Hi Beatrix, >>> >>> The solution I propose involves introducing the hotpatch I attached >>> to fix electric-layout-mode in your emacs, so I wouldn't expect it >>> to work if you haven't done that. >>> >>> Do you know how to do it? >>> >>> Though Alan will probably suggest otherwise, I'd also steer away >>> from c-specific functionality and keep to the triad >>> electric-indent-mode, electric-pair-mode and electric-indent-mode, >>> at least while we try to extend/fix these modes to accommodate your >>> needs. >>> >>> After such a solution is evaluated, you can select to keep it or move to something else. >>> >>> João >>> >>> On Fri, Dec 21, 2018, 19:06 Beatrix Klebe <beeuhtricks <at> gmail.com wrote: >>>> >>>> Here's the link, I believe it was Stefan that answered it: >>>> https://emacs.stackexchange.com/questions/2837/automatically-formatting-brackets/2853#2853 >>>> >>>> I have tried this with emacs -Q and it does not fix the issue, which >>>> is as follows. >>>> >>>> Ordinarily in cc-mode when you have auto-newline-mode activated, and >>>> as far as I can tell, a cc-mode configuration that supports it, (which >>>> csharp-mode contains), the following happens when opening a block >>>> (pipe is the cursor): >>>> >>>> void Main() {| // opening bracket is typed >>>> >>>> becomes >>>> >>>> void Main >>>> { >>>> | >>>> >>>> when c-toggle-auto-newline is activated. However, if you also want >>>> your braces automatically paired, with electric-pair-mode, instead the >>>> following occurs: >>>> >>>> void Main() {| // opening bracket is typed >>>> >>>> void Main() {|} // electric-pair-mode closes the open bracket, but >>>> auto-newline-mode does not appear to do anything. >>>> >>>> void Main() { >>>> | >>>> } // user hits return, inserting the cursor at the correct indent >>>> level, but leaving the opening brace where it is. >>>> >>>> The ideal/desired behavior is: >>>> >>>> void Main() {| // opening bracket is typed >>>> >>>> void Main() >>>> { >>>> | >>>> } // user hits return key, electric-pair-mode pairs up the brackets, >>>> and auto-newline-mode formats the braces correctly >>>> >>>> It would also probably suffice to format with the newline before >>>> hitting enter as well, although I think I prefer hitting enter to open >>>> the block. I'm quite curious as to the internals of these formatting >>>> systems and would be happy to help with a fix/feature if that would be >>>> desired, I am mostly an OCaml programmer but C# is my day job and I've >>>> just recently gotten deeper into Emacs Lisp. >>>> >>>> On Fri, Dec 21, 2018 at 1:49 PM João Távora <joaotavora <at> gmail.com> wrote: >>>> > >>>> > Beatrix Klebe <beeuhtricks <at> gmail.com> writes: >>>> > >>>> > > I believe I saw your Stack Overflow answer about this while searching >>>> > > for the solution. electric-layout-mode works with some quirks, such as >>>> > > that if you put a space after parens in a function definition, the >>>> > > space gets carried on to the newline with that method, which is a bit >>>> > > annoying. What would be ideal, and what I'm looking for, is to get >>>> > > auto-pairing of brackets with braces being placed where they should be >>>> > > automatically and the insertion point getting put in between them at >>>> > > the correct indent level, such as what happens with Visual Studio, or >>>> > > Visual Studio Code, or several other editors with this functionality. >>>> > > Perhaps it is not emacslike to have such behavior be totally >>>> > > automated, but I am used to it and finds it decreases my ordinary >>>> > > levels of frustration when working with verbose and imperative >>>> > > languages. I am currently trying to write some insert specifiers for >>>> > > smartparens to do this, but it is proving more difficult to find an >>>> > > elegant solution than I had expected. >>>> > >>>> > It is quite emacslike (though maybe not activated by default): you just >>>> > have to report the bugs to the Emacs developers as efficiently as >>>> > possible. >>>> > >>>> > 1. Though Alan possibly has already, I still cannot understand the >>>> > original problem. Can you start by describing what the buffer looked >>>> > like before, what you did, what it looked like afterwards, and what >>>> > you expected it to look like? If possible start with a clean Emacs >>>> > -Q recpe. >>>> > >>>> > 2. I have experimented with nicer-playing like alternatives like >>>> > electric-layout-mode. I came across a few quirks myself (though I'm >>>> > not sure if they are the same as yours). So I prepared a patch (in >>>> > branch scratch/fix-33794-extend-electric-layout-mode) and attached >>>> > it after the sig. >>>> > >>>> > After loading this patch, in a simple Emacs -Q the configuration: >>>> > >>>> > (electric-pair-mode) >>>> > (electric-layout-mode) >>>> > >>>> > (add-hook 'c-mode-hook >>>> > (lambda () >>>> > (setq-local electric-layout-rules >>>> > '((?\{ . after) >>>> > (?\{ . after-stay))))) >>>> > >>>> > And, when visiting a C file, if I press `{' I get the expected >>>> > pair+layout+indent behaviour. Sor example opening a brace after >>>> > int main () gives me: >>>> > >>>> > int main () { >>>> > <cursor here> >>>> > } >>>> > >>>> > I, like Stefan, think cc-mode could/should set electric-layout-rules >>>> > buffer-locally to reflect whatever c-style the user has selected. >>>> > >>>> > Thanks, >>>> > João >>>> > >>>> > PS: Also, can you link to the the relevant to the stack overflow answer you >>>> > mentioned? >>>> > >>>> > commit ab036bdedbb49ecc96d550b5e883e43bb03eaccc >>>> > Author: João Távora <joaotavora <at> gmail.com> >>>> > Date: Fri Dec 21 18:00:08 2018 +0000 >>>> > >>>> > Extend electric-layout-mode to handle more complex layouts >>>> > >>>> > Also, have it play nice with electric-pair-mode. >>>> > >>>> > Multiple matching entries in `electric-layout-rules' are executed in >>>> > order of appearance. When inserting a newline in the 'after-stay >>>> > rule, ensure electric-pair-open-newline-between-pairs is nil. >>>> > >>>> > Arguably the logic behind electric-pair-open-newline-between-pairs >>>> > should be moved to electric-layout-mode, but the current rule-matching >>>> > engine doesn't allow for it. The current solution seems to be good >>>> > enough for the situations reported in bug#33794. >>>> > >>>> > * lisp/electric.el (electric-layout-rules): Adjust docstring. >>>> > (electric-layout-post-self-insert-function): Loop through rules. Bind >>>> > electric-pair-open-newline-between-pairs to nil when handling >>>> > after-stay. >>>> > >>>> > diff --git a/lisp/electric.el b/lisp/electric.el >>>> > index 6dbf46b80c..6a307a49b9 100644 >>>> > --- a/lisp/electric.el >>>> > +++ b/lisp/electric.el >>>> > @@ -370,38 +370,43 @@ electric-layout-rules >>>> > >>>> > The symbols specify where in relation to CHAR the newline >>>> > character(s) should be inserted. `after-stay' means insert a >>>> > -newline after CHAR but stay in the same place.") >>>> > +newline after CHAR but stay in the same place. >>>> > + >>>> > +If multiple rules match, they are all executed in order of >>>> > +appearance.") >>>> > >>>> > (defun electric-layout-post-self-insert-function () >>>> > - (let* ((rule (cdr (assq last-command-event electric-layout-rules))) >>>> > - pos) >>>> > - (when (and rule >>>> > - (setq pos (electric--after-char-pos)) >>>> > + (let (pos) >>>> > + (when (and (setq pos (electric--after-char-pos)) >>>> > ;; Not in a string or comment. >>>> > (not (nth 8 (save-excursion (syntax-ppss pos))))) >>>> > - (let ((end (point-marker)) >>>> > - (sym (if (functionp rule) (funcall rule) rule))) >>>> > - (set-marker-insertion-type end (not (eq sym 'after-stay))) >>>> > - (goto-char pos) >>>> > - (pcase sym >>>> > - ;; FIXME: we used `newline' down here which called >>>> > - ;; self-insert-command and ran post-self-insert-hook recursively. >>>> > - ;; It happened to make electric-indent-mode work automatically with >>>> > - ;; electric-layout-mode (at the cost of re-indenting lines >>>> > - ;; multiple times), but I'm not sure it's what we want. >>>> > - ;; >>>> > - ;; FIXME: check eolp before inserting \n? >>>> > - ('before (goto-char (1- pos)) (skip-chars-backward " \t") >>>> > - (unless (bolp) (insert "\n"))) >>>> > - ('after (insert "\n")) >>>> > - ('after-stay (save-excursion >>>> > - (let ((electric-layout-rules nil)) >>>> > - (newline 1 t)))) >>>> > - ('around (save-excursion >>>> > - (goto-char (1- pos)) (skip-chars-backward " \t") >>>> > - (unless (bolp) (insert "\n"))) >>>> > - (insert "\n"))) ; FIXME: check eolp before inserting \n? >>>> > - (goto-char end))))) >>>> > + (goto-char pos) >>>> > + (dolist (rule electric-layout-rules) >>>> > + (when (eq last-command-event (car rule)) >>>> > + (let* ((end (point-marker)) >>>> > + (rule (cdr rule)) >>>> > + (sym (if (functionp rule) (funcall rule) rule))) >>>> > + (set-marker-insertion-type end (not (eq sym 'after-stay))) >>>> > + (pcase sym >>>> > + ;; FIXME: we used `newline' down here which called >>>> > + ;; self-insert-command and ran post-self-insert-hook recursively. >>>> > + ;; It happened to make electric-indent-mode work automatically with >>>> > + ;; electric-layout-mode (at the cost of re-indenting lines >>>> > + ;; multiple times), but I'm not sure it's what we want. >>>> > + ;; >>>> > + ;; FIXME: check eolp before inserting \n? >>>> > + ('before (goto-char (1- pos)) (skip-chars-backward " \t") >>>> > + (unless (bolp) (insert "\n"))) >>>> > + ('after (insert "\n")) >>>> > + ('after-stay (save-excursion >>>> > + (let ((electric-layout-rules nil) >>>> > + (electric-pair-open-newline-between-pairs nil)) >>>> > + (newline 1 t)))) >>>> > + ('around (save-excursion >>>> > + (goto-char (1- pos)) (skip-chars-backward " \t") >>>> > + (unless (bolp) (insert "\n"))) >>>> > + (insert "\n"))) ; FIXME: check eolp before inserting \n? >>>> > + (goto-char end))))))) >>>> > >>>> > (put 'electric-layout-post-self-insert-function 'priority 40) >>>> >
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.