unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#16346: 24.3; electric-pair-mode close-paren issue
@ 2014-01-05  2:57 Leo Liu
  2014-01-05 11:49 ` João Távora
  0 siblings, 1 reply; 17+ messages in thread
From: Leo Liu @ 2014-01-05  2:57 UTC (permalink / raw)
  To: 16346; +Cc: joaotavora

In some of the eldoc-documentation-function's when used by
eldoc-post-insert-mode, for example octave-eldoc-function, the eldoc
(calltip) is only shown after inserting the ( and inside the ().

The new feature introduced to electric-pair-mode fails the above by
triggering two successive runs of post-self-insert-hook, one for
inserting the open-paren and the other for the close-paren. Thus the
eldoc message is immediately cleared by the second run.

My impression is if a user didn't type ) it should not trigger
post-self-insert-hook. WDYT?

Leo





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-05  2:57 bug#16346: 24.3; electric-pair-mode close-paren issue Leo Liu
@ 2014-01-05 11:49 ` João Távora
  2014-01-05 15:30   ` Leo Liu
  0 siblings, 1 reply; 17+ messages in thread
From: João Távora @ 2014-01-05 11:49 UTC (permalink / raw)
  To: Leo Liu; +Cc: 16346

Leo Liu <sdl.web@gmail.com> writes:

> In some of the eldoc-documentation-function's when used by
> eldoc-post-insert-mode, for example octave-eldoc-function, the eldoc
> (calltip) is only shown after inserting the ( and inside the ().
>
> The new feature introduced to electric-pair-mode fails the above by
> triggering two successive runs of post-self-insert-hook, one for
> inserting the open-paren and the other for the close-paren. Thus the
> eldoc message is immediately cleared by the second run.

Are you sure this is only with the new features?

> My impression is if a user didn't type ) it should not trigger
> post-self-insert-hook. WDYT?

The idea of inserting matchers with
self-insert-command is to allow other hooks to run recursively, but it
has some problems as described by the FIXME's that I found before my
changes electric-pair-mode.

Indeed, it wasn't "self inserted" it, it was "electric
inserted". electric-pair--insert is this abstraction and it's used to
insert matchers. It does some rebinding of vars to keep other modes like
blink-matching-paren from interfering. Maybe eldoc-mode deserves a place
in there?

   (defun electric-pair--insert (char)
     (let ((last-command-event char)
           (blink-matching-paren nil)
           (electric-pair-mode nil)
           (eldoc-mode nil))
       (self-insert-command 1)))

Haven't tested though... Will have little time the coming weeks, sorry.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-05 11:49 ` João Távora
@ 2014-01-05 15:30   ` Leo Liu
  2014-01-05 19:25     ` João Távora
  2014-01-05 23:13     ` Stefan Monnier
  0 siblings, 2 replies; 17+ messages in thread
From: Leo Liu @ 2014-01-05 15:30 UTC (permalink / raw)
  To: João Távora; +Cc: 16346

On 2014-01-05 19:49 +0800, João Távora wrote:
> Are you sure this is only with the new features?

Yes.

>> My impression is if a user didn't type ) it should not trigger
>> post-self-insert-hook. WDYT?
>
> The idea of inserting matchers with
> self-insert-command is to allow other hooks to run recursively, but it
> has some problems as described by the FIXME's that I found before my
> changes electric-pair-mode.

But this may break the other half of stuff using post-self-insert-hook?

> Indeed, it wasn't "self inserted" it, it was "electric
> inserted". electric-pair--insert is this abstraction and it's used to
> insert matchers. It does some rebinding of vars to keep other modes like
> blink-matching-paren from interfering. Maybe eldoc-mode deserves a place
> in there?
>
>    (defun electric-pair--insert (char)
>      (let ((last-command-event char)
>            (blink-matching-paren nil)
>            (electric-pair-mode nil)
>            (eldoc-mode nil))
>        (self-insert-command 1)))
>
> Haven't tested though... Will have little time the coming weeks, sorry.

Add (eldoc-post-insert-mode nil) fixes this bug.

Leo





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-05 15:30   ` Leo Liu
@ 2014-01-05 19:25     ` João Távora
  2014-01-05 23:13     ` Stefan Monnier
  1 sibling, 0 replies; 17+ messages in thread
From: João Távora @ 2014-01-05 19:25 UTC (permalink / raw)
  To: Leo Liu; +Cc: 16346

Leo Liu <sdl.web@gmail.com> writes:
>> Are you sure this is only with the new features?
> Yes.

Funny, `electric-pair--insert' was also being called with the previous
`electric-pair-mode' to insert matchers and it didn't change. The
previous version did sometimes match ( didn't it? :)

> But this may break the other half of stuff using
> post-self-insert-hook?

Yes, more or less I think that was the gist of the FIXME notes that
Stefan had added there. OTOH, it's useful, for example, when newlines
are inserted. electric-indent-mode has a chance to kick in there.

Let's look at this this way, if the user pressed `(', `)' and `C-b' in
sucession, what would that "half of stuff" do? This is what
electric-pair more or less emulates.

On the other hand, we can decide to insert matchers using simple
`insert'. Stefan any thoughts?

> Add (eldoc-post-insert-mode nil) fixes this bug.

Nice, looks good, maybe you could commit this and maybe add a test or
two. I can only look at this at the end of the week, sorry.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-05 15:30   ` Leo Liu
  2014-01-05 19:25     ` João Távora
@ 2014-01-05 23:13     ` Stefan Monnier
  2014-01-06  0:48       ` Leo Liu
  1 sibling, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2014-01-05 23:13 UTC (permalink / raw)
  To: Leo Liu; +Cc: 16346, João Távora

> Add (eldoc-post-insert-mode nil) fixes this bug.

FWIW, I find eldoc-post-insert-mode suffers from many problems.
I'd appreciate if someone who likes eldoc-post-insert-mode explains to
me the advantages it sees in comparison to the "default"
post-command-hook mode, so we can maybe find a better way to get the
same result.


        Stefan





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-05 23:13     ` Stefan Monnier
@ 2014-01-06  0:48       ` Leo Liu
  2014-01-09 16:12         ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Leo Liu @ 2014-01-06  0:48 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 16346, João Távora

On 2014-01-06 07:13 +0800, Stefan Monnier wrote:
> FWIW, I find eldoc-post-insert-mode suffers from many problems.
> I'd appreciate if someone who likes eldoc-post-insert-mode explains to
> me the advantages it sees in comparison to the "default"
> post-command-hook mode, so we can maybe find a better way to get the
> same result.

I might be the only one using it (exclusively) :( but I am open to
change.

The idea behind eldoc-post-insert-mode is not to show the eldoc messages
all the time which can be kinda distracting. Some editing environments
(maybe Matlab) also only show the arglist after inserting (.

In general I find the arglist (eldoc) most useful when editing text.
When browsing code it is most of the time easy to guess from context.

Also getting the arglist can be expensive. For example in octave it has
to ask the running process (which can get stuck when the process is in
the middle of doing something else). In other cases it has to make
remote calls.

Leo





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-06  0:48       ` Leo Liu
@ 2014-01-09 16:12         ` Stefan Monnier
  2014-01-10  3:24           ` Leo Liu
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2014-01-09 16:12 UTC (permalink / raw)
  To: Leo Liu; +Cc: 16346, João Távora

> The idea behind eldoc-post-insert-mode is not to show the eldoc messages
> all the time which can be kinda distracting. Some editing environments
> (maybe Matlab) also only show the arglist after inserting (.

But you can get the same result with suitable use of eldoc-remove-command.

> In general I find the arglist (eldoc) most useful when editing text.

Agreed.  But I usually need it *just before* typing.  IOW, I navigate to
the point where I want to write, then look at the eldoc info, then start
typing.  Sometimes the navigation is not in eldoc-message-commands, so
I need to do something like "C-b C-f" to get eldoc to come up, but with
eldoc-post-insert-mode it's worse, because the obvious "equivalent" of
"SPC DEL" does not end with a self-insert so I need to do SPC, then read
eldoc, then DEL which I find even more annoying.

> Also getting the arglist can be expensive. For example in octave it has
> to ask the running process (which can get stuck when the process is in
> the middle of doing something else). In other cases it has to make
> remote calls.

Not sure why that's relevant.


        Stefan





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-09 16:12         ` Stefan Monnier
@ 2014-01-10  3:24           ` Leo Liu
  2014-01-10  4:11             ` Leo Liu
  2014-01-10 14:14             ` Stefan Monnier
  0 siblings, 2 replies; 17+ messages in thread
From: Leo Liu @ 2014-01-10  3:24 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 16346, João Távora

On 2014-01-10 00:12 +0800, Stefan Monnier wrote:
> But you can get the same result with suitable use of eldoc-remove-command.

But in case of a read-only buffer, one may want the normal eldoc to show
arglist. So there is two use cases: one when editing and the other when
reading others code. I only enable eldoc-mode manually i.e. M-x
eldoc-mode when I need it.

>> In general I find the arglist (eldoc) most useful when editing text.
>
> Agreed.  But I usually need it *just before* typing.  IOW, I navigate to
> the point where I want to write, then look at the eldoc info, then start
> typing.  Sometimes the navigation is not in eldoc-message-commands, so
> I need to do something like "C-b C-f" to get eldoc to come up, but with
> eldoc-post-insert-mode it's worse, because the obvious "equivalent" of
> "SPC DEL" does not end with a self-insert so I need to do SPC, then read
> eldoc, then DEL which I find even more annoying.

But then when you go anywhere that you don't want to edit code (such as
just to copy some text) eldoc also prints the arglist. The latter
happens more often in my editing habit that it can be annoying.

But maybe eldoc-post-insert-mode (maybe even a new name
eldoc-edit-mode?) can check on char changes instead? Is this better?

>> Also getting the arglist can be expensive. For example in octave it has
>> to ask the running process (which can get stuck when the process is in
>> the middle of doing something else). In other cases it has to make
>> remote calls.
>
> Not sure why that's relevant.

For example, if a heavy job is running in the inferior octave buffer,
one normally don't want any movement to send a request to it asking for
arglist. It is my impression that eldoc might do that.

Leo





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-10  3:24           ` Leo Liu
@ 2014-01-10  4:11             ` Leo Liu
  2014-01-10 14:14             ` Stefan Monnier
  1 sibling, 0 replies; 17+ messages in thread
From: Leo Liu @ 2014-01-10  4:11 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 16346

On 2014-01-10 11:24 +0800, Leo Liu wrote:
> But maybe eldoc-post-insert-mode (maybe even a new name
> eldoc-edit-mode?) can check on char changes instead? Is this better?

Stefan, do you think this is better? With the following patch,
eldoc-edit-mode no longer depends on post-self-insert-hook.

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 3a7f08f5..1da6c0ab 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -173,19 +173,29 @@ (define-minor-mode eldoc-mode
    (remove-hook 'post-command-hook 'eldoc-schedule-timer)
    (remove-hook 'pre-command-hook 'eldoc-pre-command-refresh-echo-area)))
 
+(defvar-local eldoc-edit-should-print nil)
+
+(defun eldoc-after-change (beg end len)
+  (and (or (> len 0) (and (zerop len) (> end beg)))
+       (setq eldoc-edit-should-print t)))
+
 ;;;###autoload
-(define-minor-mode eldoc-post-insert-mode nil
+(define-minor-mode eldoc-edit-mode nil
   :group 'eldoc :lighter (:eval (if eldoc-mode ""
-				  (concat eldoc-minor-mode-string "|i")))
+				  (concat eldoc-minor-mode-string "|e")))
   (setq eldoc-last-message nil)
   (let ((prn-info (lambda ()
-		    (unless eldoc-mode
-		      (eldoc-print-current-symbol-info)))))
-    (if eldoc-post-insert-mode
-	(add-hook 'post-self-insert-hook prn-info nil t)
-      (remove-hook 'post-self-insert-hook prn-info t))))
+		    (when (and (not eldoc-mode) eldoc-edit-should-print)
+		      (ignore-errors (eldoc-print-current-symbol-info))
+		      (setq eldoc-edit-should-print nil)))))
+    (if eldoc-edit-mode
+	(progn
+	  (add-hook 'post-command-hook prn-info nil t)
+	  (add-hook 'after-change-functions 'eldoc-after-change nil t))
+      (remove-hook 'post-command-hook prn-info t)
+      (remove-hook 'after-change-functions 'eldoc-after-change t))))
 
-(add-hook 'eval-expression-minibuffer-setup-hook 'eldoc-post-insert-mode)
+(add-hook 'eval-expression-minibuffer-setup-hook 'eldoc-edit-mode)
 
 ;;;###autoload
 (defun turn-on-eldoc-mode ()
@@ -309,7 +319,7 @@ (defvar eldoc-documentation-function nil
 
 (defun eldoc-print-current-symbol-info ()
   (condition-case err
-      (and (or (eldoc-display-message-p) eldoc-post-insert-mode)
+      (and (or (eldoc-display-message-p) eldoc-edit-mode)
 	   (if eldoc-documentation-function
 	       (eldoc-message (funcall eldoc-documentation-function))
 	     (let* ((current-symbol (eldoc-current-symbol))





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-10  3:24           ` Leo Liu
  2014-01-10  4:11             ` Leo Liu
@ 2014-01-10 14:14             ` Stefan Monnier
  2014-01-10 16:46               ` Leo Liu
  1 sibling, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2014-01-10 14:14 UTC (permalink / raw)
  To: Leo Liu; +Cc: 16346, João Távora

>> But you can get the same result with suitable use of eldoc-remove-command.
> But in case of a read-only buffer, one may want the normal eldoc to show
> arglist.  So there is two use cases: one when editing and the other when
> reading others code.  I only enable eldoc-mode manually i.e. M-x
> eldoc-mode when I need it.

So you want an eldoc-subdued-mode which only gives you info in some very
restricted cases (e.g. self-insert-command) and which you can
dynamically enable/disable in specific buffers.

Making eldoc-message-commands buffer-local would let us do that fairly easily.

> But then when you go anywhere that you don't want to edit code (such as
> just to copy some text) eldoc also prints the arglist.

Right.  And it doesn't bother me because if I don't want to see the
eldoc info, I just don't look at the echo area.

> The latter happens more often in my editing habit that it can
> be annoying.

We obviously have different tastes/habits ;-)

> But maybe eldoc-post-insert-mode (maybe even a new name
> eldoc-edit-mode?) can check on char changes instead?

I don't understand what you mean by "char changes".
Ah, you mean using an after-change-function?  I don't really like that.
I'm not sure what problem it is supposed to avoid.

>>> Also getting the arglist can be expensive. For example in octave it has
>>> to ask the running process (which can get stuck when the process is in
>>> the middle of doing something else). In other cases it has to make
>>> remote calls.
>> Not sure why that's relevant.
> For example, if a heavy job is running in the inferior octave buffer,
> one normally don't want any movement to send a request to it asking for
> arglist.

But neither do you want to send such a request just because you inserted
a char.  So, the problem really applies to bother cases.  I find it hard
to believe that the problem would really be much more serious in one
case then in the other: either it's serious in both cases, or it's
bearable in both cases.


        Stefan


PS: I think some version of eldoc-mode should be enabled by default.
It's too late for 24.4, but: for the one after.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-10 14:14             ` Stefan Monnier
@ 2014-01-10 16:46               ` Leo Liu
  2014-01-10 17:20                 ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Leo Liu @ 2014-01-10 16:46 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 16346

On 2014-01-10 22:14 +0800, Stefan Monnier wrote:
> So you want an eldoc-subdued-mode which only gives you info in some very
> restricted cases (e.g. self-insert-command) and which you can
> dynamically enable/disable in specific buffers.
>
> Making eldoc-message-commands buffer-local would let us do that fairly easily.
>
>> But then when you go anywhere that you don't want to edit code (such as
>> just to copy some text) eldoc also prints the arglist.
>
> Right.  And it doesn't bother me because if I don't want to see the
> eldoc info, I just don't look at the echo area.

I haven't built that habit. If there is nothing to show in the status
area show nothing which is what I expect. In practice, even if my eyes
don't literally look at the echo area, they still grab a bit of my
attention.

> We obviously have different tastes/habits ;-)
>
>> But maybe eldoc-post-insert-mode (maybe even a new name
>> eldoc-edit-mode?) can check on char changes instead?
>
> I don't understand what you mean by "char changes".
> Ah, you mean using an after-change-function?  I don't really like that.
> I'm not sure what problem it is supposed to avoid.

The SPC DEL thing that you mentioned and this bug.

[snipped 9 lines]
> But neither do you want to send such a request just because you inserted
> a char.  So, the problem really applies to bother cases.  I find it hard
> to believe that the problem would really be much more serious in one
> case then in the other: either it's serious in both cases, or it's
> bearable in both cases.
>
>
>         Stefan
>
>
> PS: I think some version of eldoc-mode should be enabled by default.
> It's too late for 24.4, but: for the one after.

OK I don't mind taking eldoc-post-insert-mode out completely. Let me
know if you want me to. It is new in 24.4 so better go now if it has to
go. I can put it in my init file if I need it.

Leo





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-10 16:46               ` Leo Liu
@ 2014-01-10 17:20                 ` Stefan Monnier
  2014-01-11  4:38                   ` Leo Liu
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2014-01-10 17:20 UTC (permalink / raw)
  To: Leo Liu; +Cc: 16346

>>> But maybe eldoc-post-insert-mode (maybe even a new name
>>> eldoc-edit-mode?) can check on char changes instead?
>> I don't understand what you mean by "char changes".
>> Ah, you mean using an after-change-function?  I don't really like that.
>> I'm not sure what problem it is supposed to avoid.
> The SPC DEL thing that you mentioned

Ah, right, it would cause the DEL to also emit the eldoc info.

> and this bug.

Using a buffer-local eldoc-message-commands which only contains
self-insert-command would also fix this bug.

> OK I don't mind taking eldoc-post-insert-mode out completely.  Let me
> know if you want me to.  It is new in 24.4 so better go now if it has to
> go.  I can put it in my init file if I need it.

I think the idea is fine, but using post-self-insert-hook turns out to
be a bad idea, I think (tho I liked it, initially).
Could you try and re-implement it as a minor mode that enables
eldoc-mode and tweaks eldoc-message-commands buffer-locally, to see if
that works better?


        Stefan





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-10 17:20                 ` Stefan Monnier
@ 2014-01-11  4:38                   ` Leo Liu
  2014-01-11  5:35                     ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Leo Liu @ 2014-01-11  4:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 16346

On 2014-01-11 01:20 +0800, Stefan Monnier wrote:
> I think the idea is fine, but using post-self-insert-hook turns out to
> be a bad idea, I think (tho I liked it, initially).
> Could you try and re-implement it as a minor mode that enables
> eldoc-mode and tweaks eldoc-message-commands buffer-locally, to see if
> that works better?

The following patch seems to do it. Comments?

=== modified file 'lisp/emacs-lisp/eldoc.el'
--- lisp/emacs-lisp/eldoc.el	2014-01-06 23:34:05 +0000
+++ lisp/emacs-lisp/eldoc.el	2014-01-11 04:31:35 +0000
@@ -62,6 +62,11 @@
   :type 'number
   :group 'eldoc)
 
+(defcustom eldoc-print-after-edit nil
+  "If non-nil eldoc info is only shown when editing."
+  :type 'boolean
+  :group 'eldoc)
+
 ;;;###autoload
 (defcustom eldoc-minor-mode-string (purecopy " ElDoc")
   "String to display in mode line when ElDoc Mode is enabled; nil for none."
@@ -150,6 +155,16 @@
   "The function used by `eldoc-message' to display messages.
 It should receive the same arguments as `message'.")
 
+(defvar eldoc-edit-message-commands
+  (let ((cmds (make-vector 31 0))
+	(re (regexp-opt '("delete" "insert" "edit" "electric" "newline"))))
+    (mapatoms (lambda (s)
+		(and (commandp s)
+		     (string-match re (symbol-name s))
+		     (intern (symbol-name s) cmds)))
+	      obarray)
+    cmds))
+
 \f
 ;;;###autoload
 (define-minor-mode eldoc-mode
@@ -168,25 +183,13 @@
   (setq eldoc-last-message nil)
   (if eldoc-mode
       (progn
+	(when eldoc-print-after-edit
+	  (setq-local eldoc-message-commands eldoc-edit-message-commands))
 	(add-hook 'post-command-hook 'eldoc-schedule-timer nil t)
 	(add-hook 'pre-command-hook 'eldoc-pre-command-refresh-echo-area t))
-   (remove-hook 'post-command-hook 'eldoc-schedule-timer)
-   (remove-hook 'pre-command-hook 'eldoc-pre-command-refresh-echo-area)))
-
-;;;###autoload
-(define-minor-mode eldoc-post-insert-mode nil
-  :group 'eldoc :lighter (:eval (if eldoc-mode ""
-				  (concat eldoc-minor-mode-string "|i")))
-  (setq eldoc-last-message nil)
-  (let ((prn-info (lambda ()
-		    (unless eldoc-mode
-		      (eldoc-print-current-symbol-info)))))
-    (if eldoc-post-insert-mode
-	(add-hook 'post-self-insert-hook prn-info nil t)
-      (remove-hook 'post-self-insert-hook prn-info t))))
-
-;; FIXME: This changes Emacs's behavior when the file is loaded!
-(add-hook 'eval-expression-minibuffer-setup-hook 'eldoc-post-insert-mode)
+    (kill-local-variable 'eldoc-message-commands)
+    (remove-hook 'post-command-hook 'eldoc-schedule-timer t)
+    (remove-hook 'pre-command-hook 'eldoc-pre-command-refresh-echo-area t)))
 
 ;;;###autoload
 (defun turn-on-eldoc-mode ()
@@ -264,8 +267,10 @@
 ;; This doesn't seem to be required for Emacs 19.28 and earlier.
 (defun eldoc-pre-command-refresh-echo-area ()
   (and eldoc-last-message
-       (if (eldoc-display-message-no-interference-p)
-           (eldoc-message eldoc-last-message)
+       (if (and (eldoc-display-message-no-interference-p)
+		(symbolp this-command)
+		(intern-soft (symbol-name this-command) eldoc-message-commands))
+	   (eldoc-message eldoc-last-message)
          (setq eldoc-last-message nil))))
 
 ;; Decide whether now is a good time to display a message.
@@ -283,9 +288,7 @@
 ;; Check various conditions about the current environment that might make
 ;; it undesirable to print eldoc messages right this instant.
 (defun eldoc-display-message-no-interference-p ()
-  (and eldoc-mode
-       (not executing-kbd-macro)
-       (not (and (boundp 'edebug-active) edebug-active))))
+  (not (or executing-kbd-macro (bound-and-true-p edebug-active))))
 
 \f
 ;;;###autoload
@@ -309,7 +312,7 @@
   ;; This is run from post-command-hook or some idle timer thing,
   ;; so we need to be careful that errors aren't ignored.
   (with-demoted-errors "eldoc error: %s"
-    (and (or (eldoc-display-message-p) eldoc-post-insert-mode)
+    (and (eldoc-display-message-p)
 	 (if eldoc-documentation-function
 	     (eldoc-message (funcall eldoc-documentation-function))
 	   (let* ((current-symbol (eldoc-current-symbol))

=== modified file 'lisp/simple.el'
--- lisp/simple.el	2014-01-09 01:59:19 +0000
+++ lisp/simple.el	2014-01-11 04:28:32 +0000
@@ -1387,6 +1387,7 @@
         (lambda ()
           (add-hook 'completion-at-point-functions
                     #'lisp-completion-at-point nil t)
+	  (eldoc-mode 1)
           (run-hooks 'eval-expression-minibuffer-setup-hook))
       (read-from-minibuffer prompt initial-contents
                             read-expression-map t






^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-11  4:38                   ` Leo Liu
@ 2014-01-11  5:35                     ` Stefan Monnier
  2014-01-11  6:11                       ` Leo Liu
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2014-01-11  5:35 UTC (permalink / raw)
  To: Leo Liu; +Cc: 16346

>> I think the idea is fine, but using post-self-insert-hook turns out to
>> be a bad idea, I think (tho I liked it, initially).
>> Could you try and re-implement it as a minor mode that enables
>> eldoc-mode and tweaks eldoc-message-commands buffer-locally, to see if
>> that works better?
> The following patch seems to do it.  Comments?

Looks good from here.  How do you like its behavior?


        Stefan





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-11  5:35                     ` Stefan Monnier
@ 2014-01-11  6:11                       ` Leo Liu
  2014-01-12  3:35                         ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Leo Liu @ 2014-01-11  6:11 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 16346

On 2014-01-11 13:35 +0800, Stefan Monnier wrote:
> Looks good from here.  How do you like its behavior?
>
>
>         Stefan

Seems to work well for me.

If you have no objection I will install the following final patch. Have
to build eldoc-edit-message-commands at the time of enabling eldoc-mode
because loading packages may add new edit commands. (time penalty is
about 0.03s, hope this isn't too bad.)

=== modified file 'lisp/emacs-lisp/eldoc.el'
--- lisp/emacs-lisp/eldoc.el	2014-01-06 23:34:05 +0000
+++ lisp/emacs-lisp/eldoc.el	2014-01-11 06:10:50 +0000
@@ -62,6 +62,12 @@
   :type 'number
   :group 'eldoc)
 
+(defcustom eldoc-print-after-edit nil
+  "If non-nil eldoc info is only shown when editing.
+Changing the value requires toggling `eldoc-mode'."
+  :type 'boolean
+  :group 'eldoc)
+
 ;;;###autoload
 (defcustom eldoc-minor-mode-string (purecopy " ElDoc")
   "String to display in mode line when ElDoc Mode is enabled; nil for none."
@@ -150,6 +156,16 @@
   "The function used by `eldoc-message' to display messages.
 It should receive the same arguments as `message'.")
 
+(defun eldoc-edit-message-commands ()
+  (let ((cmds (make-vector 31 0))
+	(re (regexp-opt '("delete" "insert" "edit" "electric" "newline"))))
+    (mapatoms (lambda (s)
+		(and (commandp s)
+		     (string-match re (symbol-name s))
+		     (intern (symbol-name s) cmds)))
+	      obarray)
+    cmds))
+
 \f
 ;;;###autoload
 (define-minor-mode eldoc-mode
@@ -168,25 +184,13 @@
   (setq eldoc-last-message nil)
   (if eldoc-mode
       (progn
+	(when eldoc-print-after-edit
+	  (setq-local eldoc-message-commands (eldoc-edit-message-commands)))
 	(add-hook 'post-command-hook 'eldoc-schedule-timer nil t)
 	(add-hook 'pre-command-hook 'eldoc-pre-command-refresh-echo-area t))
-   (remove-hook 'post-command-hook 'eldoc-schedule-timer)
-   (remove-hook 'pre-command-hook 'eldoc-pre-command-refresh-echo-area)))
-
-;;;###autoload
-(define-minor-mode eldoc-post-insert-mode nil
-  :group 'eldoc :lighter (:eval (if eldoc-mode ""
-				  (concat eldoc-minor-mode-string "|i")))
-  (setq eldoc-last-message nil)
-  (let ((prn-info (lambda ()
-		    (unless eldoc-mode
-		      (eldoc-print-current-symbol-info)))))
-    (if eldoc-post-insert-mode
-	(add-hook 'post-self-insert-hook prn-info nil t)
-      (remove-hook 'post-self-insert-hook prn-info t))))
-
-;; FIXME: This changes Emacs's behavior when the file is loaded!
-(add-hook 'eval-expression-minibuffer-setup-hook 'eldoc-post-insert-mode)
+    (kill-local-variable 'eldoc-message-commands)
+    (remove-hook 'post-command-hook 'eldoc-schedule-timer t)
+    (remove-hook 'pre-command-hook 'eldoc-pre-command-refresh-echo-area t)))
 
 ;;;###autoload
 (defun turn-on-eldoc-mode ()
@@ -264,8 +268,10 @@
 ;; This doesn't seem to be required for Emacs 19.28 and earlier.
 (defun eldoc-pre-command-refresh-echo-area ()
   (and eldoc-last-message
-       (if (eldoc-display-message-no-interference-p)
-           (eldoc-message eldoc-last-message)
+       (if (and (eldoc-display-message-no-interference-p)
+		(symbolp this-command)
+		(intern-soft (symbol-name this-command) eldoc-message-commands))
+	   (eldoc-message eldoc-last-message)
          (setq eldoc-last-message nil))))
 
 ;; Decide whether now is a good time to display a message.
@@ -283,9 +289,7 @@
 ;; Check various conditions about the current environment that might make
 ;; it undesirable to print eldoc messages right this instant.
 (defun eldoc-display-message-no-interference-p ()
-  (and eldoc-mode
-       (not executing-kbd-macro)
-       (not (and (boundp 'edebug-active) edebug-active))))
+  (not (or executing-kbd-macro (bound-and-true-p edebug-active))))
 
 \f
 ;;;###autoload
@@ -309,7 +313,7 @@
   ;; This is run from post-command-hook or some idle timer thing,
   ;; so we need to be careful that errors aren't ignored.
   (with-demoted-errors "eldoc error: %s"
-    (and (or (eldoc-display-message-p) eldoc-post-insert-mode)
+    (and (eldoc-display-message-p)
 	 (if eldoc-documentation-function
 	     (eldoc-message (funcall eldoc-documentation-function))
 	   (let* ((current-symbol (eldoc-current-symbol))

=== modified file 'lisp/progmodes/octave.el'
--- lisp/progmodes/octave.el	2014-01-10 10:35:36 +0000
+++ lisp/progmodes/octave.el	2014-01-11 06:02:44 +0000
@@ -154,12 +154,8 @@
     ["Insert Function"              octave-insert-defun t]
     ["Update Function File Comment" octave-update-function-file-comment t]
     "---"
-    ["Function Syntax Hints" (call-interactively
-                              (if (fboundp 'eldoc-post-insert-mode)
-                                  'eldoc-post-insert-mode
-                                'eldoc-mode))
-     :style toggle :selected (or (bound-and-true-p eldoc-post-insert-mode)
-				 (bound-and-true-p eldoc-mode))
+    ["Function Syntax Hints" (eldoc-mode 'toggle)
+     :style toggle :selected (bound-and-true-p eldoc-mode)
      :help "Display function signatures after typing `SPC' or `('"]
     ["Delimiter Matching"           show-paren-mode
      :style toggle :selected show-paren-mode

=== modified file 'lisp/simple.el'
--- lisp/simple.el	2014-01-09 01:59:19 +0000
+++ lisp/simple.el	2014-01-11 04:28:32 +0000
@@ -1387,6 +1387,7 @@
         (lambda ()
           (add-hook 'completion-at-point-functions
                     #'lisp-completion-at-point nil t)
+	  (eldoc-mode 1)
           (run-hooks 'eval-expression-minibuffer-setup-hook))
       (read-from-minibuffer prompt initial-contents
                             read-expression-map t






^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-11  6:11                       ` Leo Liu
@ 2014-01-12  3:35                         ` Stefan Monnier
  2014-01-12  4:21                           ` Leo Liu
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2014-01-12  3:35 UTC (permalink / raw)
  To: Leo Liu; +Cc: 16346

> If you have no objection I will install the following final patch.

OK


        Stefan





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#16346: 24.3; electric-pair-mode close-paren issue
  2014-01-12  3:35                         ` Stefan Monnier
@ 2014-01-12  4:21                           ` Leo Liu
  0 siblings, 0 replies; 17+ messages in thread
From: Leo Liu @ 2014-01-12  4:21 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 16346-done

Fixed 24.4

On 2014-01-12 11:35 +0800, Stefan Monnier wrote:
>> If you have no objection I will install the following final patch.
>
> OK
>
>
>         Stefan

OK installed and closing this bug. Thanks for your time.

Leo





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-01-12  4:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-05  2:57 bug#16346: 24.3; electric-pair-mode close-paren issue Leo Liu
2014-01-05 11:49 ` João Távora
2014-01-05 15:30   ` Leo Liu
2014-01-05 19:25     ` João Távora
2014-01-05 23:13     ` Stefan Monnier
2014-01-06  0:48       ` Leo Liu
2014-01-09 16:12         ` Stefan Monnier
2014-01-10  3:24           ` Leo Liu
2014-01-10  4:11             ` Leo Liu
2014-01-10 14:14             ` Stefan Monnier
2014-01-10 16:46               ` Leo Liu
2014-01-10 17:20                 ` Stefan Monnier
2014-01-11  4:38                   ` Leo Liu
2014-01-11  5:35                     ` Stefan Monnier
2014-01-11  6:11                       ` Leo Liu
2014-01-12  3:35                         ` Stefan Monnier
2014-01-12  4:21                           ` Leo Liu

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).