Export pointer visibility

Export pointer visibility

[Julien Danjou]

Hi there,

I've played with mouse-avoidance-mode today, but noticed that it did
move the cursor even if it was hidden by `make-pointer-invisible'.

So there's 2 patches, one exporting the pointer visibility and another
one enhancing avoid.el to do nothing when the pointer is already
invisible.

Feedbacks welcome,

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[PATCH 1/2] Add frame-pointer-visible-p

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 doc/lispref/frames.texi |    7 +++++++
 src/ChangeLog           |    5 +++++
 src/frame.c             |   15 +++++++++++++++
 src/lisp.h              |    1 +
 4 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index d27010d..854e7b0 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -1748,6 +1748,13 @@ If @var{frame} is not visible, this function does nothing.  The return
 value is not significant.
 @end defun
 
+@defun frame-pointer-visible-p &opt frame
+This function return the current visibility status of the mouse
+pointer in @var{frame}. This is useful when
+@code{make-pointer-invisible} is set to @code{t}: it allows to know if
+the pointer has been hidden.
+@end defun
+
 @need 3000
 
 @node Pop-Up Menus
diff --git a/src/ChangeLog b/src/ChangeLog
index 5a5bcd2..48b8c9b 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2010-10-14  Julien Danjou  <julien@danjou.info>
+
+	* frame.c (Fframe_pointer_visible_p): Add
+	`frame-pointer-visible-p' to get the pointer visibility.
+
 2010-10-14  Juanma Barranquero  <lekktu@gmail.com>
 
 	* w32fns.c (w32_wnd_proc, file_dialog_callback):
diff --git a/src/frame.c b/src/frame.c
index 04cc1ca..5e32203 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -4314,6 +4314,20 @@ frame_make_pointer_visible (void)
     }
 }
 
+DEFUN ("frame-pointer-visible-p", Fframe_pointer_visible_p,
+       Sframe_pointer_visible_p, 0, 1, 0,
+       doc: /* Return FRAME pointer visibility status.
+This is useful when `make-pointer-invisible' is set.  */)
+  (Lisp_Object frame)
+{
+  if (NILP (frame))
+    frame = selected_frame;
+
+  if (frame && FRAMP (frame) && !XFRAME(frame)->pointer_invisible)
+    return Qt;
+
+  return Qnil;
+}
 
 \f
 /***********************************************************************
@@ -4623,6 +4637,7 @@ automatically.  See also `mouse-autoselect-window'.  */);
   defsubr (&Sset_frame_width);
   defsubr (&Sset_frame_size);
   defsubr (&Sset_frame_position);
+  defsubr (&Sframe_pointer_visible_p);
 
 #ifdef HAVE_WINDOW_SYSTEM
   defsubr (&Sx_get_resource);
diff --git a/src/lisp.h b/src/lisp.h
index c9104f8..a1f74cb 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3278,6 +3278,7 @@ EXFUN (Fmodify_frame_parameters, 2);
 EXFUN (Fset_frame_height, 3);
 EXFUN (Fset_frame_width, 3);
 EXFUN (Fset_frame_size, 3);
+EXFUN (Fframe_pointer_visible_p, 1);
 EXFUN (Fset_frame_position, 3);
 EXFUN (Fraise_frame, 1);
 EXFUN (Fredirect_frame_focus, 2);
-- 
1.7.1

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Stefan Monnier]

Looks pretty good, thank you, except for:

> +  if (frame && FRAMP (frame) && !XFRAME(frame)->pointer_invisible)

- Testing "frame" is wrong: it's a Lisp_Object, not a boolean (yes,
  I know Lisp_Object is often defined as `int' or `long', so the
  compiler may not complain, but it's still a bug: use
  "./configure --enable-use-lisp-union-type" to let the compiler help
  you find such bugs).

- FRAMP is an obvious typo.

- There should be a space between XFRAME and (
  
- You should add a CHECK_FRAME (frame) so as to signal an error rather
  than return nil if the arg is not a frame.

- I prefer

  return (XFRAME(frame)->pointer_invisible ? Qnil : Qt);

> +EXFUN (Fframe_pointer_visible_p, 1);

As long as the function is not called from C code, there's no need to
EXFUN it (not that it hurts, tho).

One more thing: while I see that the C code currently stores the
visibility in the frame data-structure, I'm not completely sure if this
data is truly frame-specific as opposed to terminal-specific.
Can someone confirm this issue?


        Stefan

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

On Sun, Oct 17 2010, Stefan Monnier wrote:

> Looks pretty good, thank you, except for:

Stefan, thanks a lot for the review. That's my first C patch for Emacs,
so I'm glad to see your comments and learn!

> One more thing: while I see that the C code currently stores the
> visibility in the frame data-structure, I'm not completely sure if this
> data is truly frame-specific as opposed to terminal-specific.
> Can someone confirm this issue?

In theory, since you can have more pointers nowadays, it should/can be
frame specific. Not sure it's really the case in Emacs, since Emacs
probably does not support multi-pointer right now (but I don't know for
sure).
Anyhow that may be raise a problem if you have multiple pointer on a
single frame.
Well, just thinking out loud. :)

Attached is a corrected version of the patch 1.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-frame-pointer-visible-p.patch --]
[-- Type: text/x-diff, Size: 2440 bytes --]

From ca2e3eff6772f6fb2153ff61687764d6d1444be4 Mon Sep 17 00:00:00 2001
From: Julien Danjou <julien@danjou.info>
Date: Thu, 14 Oct 2010 15:53:27 +0200
Subject: [PATCH 1/2] Add frame-pointer-visible-p

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 doc/lispref/frames.texi |    7 +++++++
 src/ChangeLog           |    5 +++++
 src/frame.c             |   14 ++++++++++++++
 3 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index d27010d..854e7b0 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -1748,6 +1748,13 @@ If @var{frame} is not visible, this function does nothing.  The return
 value is not significant.
 @end defun
 
+@defun frame-pointer-visible-p &opt frame
+This function return the current visibility status of the mouse
+pointer in @var{frame}. This is useful when
+@code{make-pointer-invisible} is set to @code{t}: it allows to know if
+the pointer has been hidden.
+@end defun
+
 @need 3000
 
 @node Pop-Up Menus
diff --git a/src/ChangeLog b/src/ChangeLog
index da344a4..95d4ad3 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -27,6 +27,11 @@
 	* font.c (Ffont_variation_glyphs):
 	* ccl.c (Fccl_execute_on_string): Fix typo in docstring.
 
+2010-10-14  Julien Danjou  <julien@danjou.info>
+
+	* frame.c (Fframe_pointer_visible_p): Add
+	`frame-pointer-visible-p' to get the pointer visibility.
+
 2010-10-14  Juanma Barranquero  <lekktu@gmail.com>
 
 	* w32fns.c (w32_wnd_proc, file_dialog_callback):
diff --git a/src/frame.c b/src/frame.c
index 04cc1ca..92c6925 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -4314,6 +4314,19 @@ frame_make_pointer_visible (void)
     }
 }
 
+DEFUN ("frame-pointer-visible-p", Fframe_pointer_visible_p,
+       Sframe_pointer_visible_p, 0, 1, 0,
+       doc: /* Return FRAME pointer visibility status.
+This is useful when `make-pointer-invisible' is set.  */)
+  (Lisp_Object frame)
+{
+  if (NILP (frame))
+    frame = selected_frame;
+
+  CHECK_FRAME (frame);
+
+  return (XFRAME (frame)->pointer_invisible ? Qnil : Qt);
+}
 
 \f
 /***********************************************************************
@@ -4623,6 +4636,7 @@ automatically.  See also `mouse-autoselect-window'.  */);
   defsubr (&Sset_frame_width);
   defsubr (&Sset_frame_size);
   defsubr (&Sset_frame_position);
+  defsubr (&Sframe_pointer_visible_p);
 
 #ifdef HAVE_WINDOW_SYSTEM
   defsubr (&Sx_get_resource);
-- 
1.7.1


[-- Attachment #3: Type: text/plain, Size: 79 bytes --]


-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Andreas Schwab]

Julien Danjou <julien@danjou.info> writes:

> +DEFUN ("frame-pointer-visible-p", Fframe_pointer_visible_p,
> +       Sframe_pointer_visible_p, 0, 1, 0,
> +       doc: /* Return FRAME pointer visibility status.

How about:
               /* Return non-nil if pointer is invisible in FRAME.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Lennart Borgman]

On Mon, Oct 18, 2010 at 11:53 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>
>> +DEFUN ("frame-pointer-visible-p", Fframe_pointer_visible_p,
>
> How about:
>               /* Return non-nil if pointer is invisible in FRAME.

Do you mean "visible"?

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Mon, 18 Oct 2010 11:53:48 +0200
> Cc: emacs-devel@gnu.org
> 
> Julien Danjou <julien@danjou.info> writes:
> 
> > +DEFUN ("frame-pointer-visible-p", Fframe_pointer_visible_p,
> > +       Sframe_pointer_visible_p, 0, 1, 0,
> > +       doc: /* Return FRAME pointer visibility status.
> 
> How about:
>                /* Return non-nil if pointer is invisible in FRAME.

"mouse pointer" is better.

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Eli Zaretskii]

> From: Julien Danjou <julien@danjou.info>
> Date: Mon, 18 Oct 2010 11:31:30 +0200
> Cc: emacs-devel@gnu.org
> 
> Stefan, thanks a lot for the review. That's my first C patch for Emacs,
> so I'm glad to see your comments and learn!

A few more below.

> > One more thing: while I see that the C code currently stores the
> > visibility in the frame data-structure, I'm not completely sure if this
> > data is truly frame-specific as opposed to terminal-specific.
> > Can someone confirm this issue?
> 
> In theory, since you can have more pointers nowadays, it should/can be
> frame specific. Not sure it's really the case in Emacs, since Emacs
> probably does not support multi-pointer right now (but I don't know for
> sure).

Isn't it possible to have the pointer invisible in one frame, then
switch to another where the pointer is visible, even if there's only
one pointer?

Anyway, as all mouse-specific variables are maintained per frame, I
don't think we should hold this one on a per-terminal basis.

> +@defun frame-pointer-visible-p &opt frame
                                  ^^^^
"&optional"

> +This function return the current visibility status of the mouse
                 ^^^^^^
"returns".  But it is better to rephrase along the lines suggested by
Andreas:

 This predicate function returns non-@code{nil} if the mouse pointer
 displayed on @var{frame} is visible; otherwise it returns
 @code{nil}.  @var{frame} omitted or @code{nil} means the selected
 frame.  This is useful when ...


> +pointer in @var{frame}. This is useful when
> +@code{make-pointer-invisible} is set to @code{t}: it allows to know if

It is a good idea to have here a cross-reference to where
make-pointer-invisible is described (in the Emacs User Manual).

> --- a/src/ChangeLog
> +++ b/src/ChangeLog
> @@ -27,6 +27,11 @@
>  	* font.c (Ffont_variation_glyphs):
>  	* ccl.c (Fccl_execute_on_string): Fix typo in docstring.
>  
> +2010-10-14  Julien Danjou  <julien@danjou.info>
> +
> +	* frame.c (Fframe_pointer_visible_p): Add
> +	`frame-pointer-visible-p' to get the pointer visibility.
> +

Your entry should be at the top of the file.

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On Mon, Oct 18 2010, Eli Zaretskii wrote:

>> +@defun frame-pointer-visible-p &opt frame
>                                   ^^^^
> "&optional"

Fixed.

>> +This function return the current visibility status of the mouse
>                  ^^^^^^
> "returns".  But it is better to rephrase along the lines suggested by
> Andreas:
>
>  This predicate function returns non-@code{nil} if the mouse pointer
>  displayed on @var{frame} is visible; otherwise it returns
>  @code{nil}.  @var{frame} omitted or @code{nil} means the selected
>  frame.  This is useful when ...

Updated.

>> +pointer in @var{frame}. This is useful when
>> +@code{make-pointer-invisible} is set to @code{t}: it allows to know if
>
> It is a good idea to have here a cross-reference to where
> make-pointer-invisible is described (in the Emacs User Manual).

Ah, now you want me to learn Texinfo. ;) I've added a @xref, hoping
that's what you meant.

> Your entry should be at the top of the file.

Fixed.

Thanks for the review Eli.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-frame-pointer-visible-p.patch --]
[-- Type: text/x-diff, Size: 2595 bytes --]

From 19fcdc3161fafefd44037c2ef6743e372a5b4692 Mon Sep 17 00:00:00 2001
From: Julien Danjou <julien@danjou.info>
Date: Thu, 14 Oct 2010 15:53:27 +0200
Subject: [PATCH 1/2] Add frame-pointer-visible-p

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 doc/lispref/frames.texi |    9 +++++++++
 src/ChangeLog           |    5 +++++
 src/frame.c             |   16 ++++++++++++++++
 3 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index d27010d..709a495 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -1748,6 +1748,15 @@ If @var{frame} is not visible, this function does nothing.  The return
 value is not significant.
 @end defun
 
+@defun frame-pointer-visible-p &optional frame
+This predicate function returns non-@code{nil} if the mouse pointer
+displayed on @var{frame} is visible; otherwise it returns @code{nil}.
+@var{frame} omitted or @code{nil} means the selected frame. This is
+useful when @code{make-pointer-invisible} is set to @code{t}: it
+allows to know if the pointer has been hidden.
+@xref{Mouse Avoidance,,,emacs}.
+@end defun
+
 @need 3000
 
 @node Pop-Up Menus
diff --git a/src/ChangeLog b/src/ChangeLog
index da344a4..f57bc43 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2010-10-18  Julien Danjou  <julien@danjou.info>
+
+	* frame.c (Fframe_pointer_visible_p): Add
+	`frame-pointer-visible-p' to get the pointer visibility.
+
 2010-10-15  Eli Zaretskii  <eliz@gnu.org>
 
 	* unexcoff.c (make_hdr): Fix prototype according to changes in
diff --git a/src/frame.c b/src/frame.c
index 04cc1ca..8a52883 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -4314,6 +4314,21 @@ frame_make_pointer_visible (void)
     }
 }
 
+DEFUN ("frame-pointer-visible-p", Fframe_pointer_visible_p,
+       Sframe_pointer_visible_p, 0, 1, 0,
+       doc: /* Returns t if the mouse pointer displayed on FRAME is visible.
+Otherwise it returns nil. FRAME omitted or nil means the
+selected frame.  This is useful when `make-pointer-invisible' is
+set.  */)
+  (Lisp_Object frame)
+{
+  if (NILP (frame))
+    frame = selected_frame;
+
+  CHECK_FRAME (frame);
+
+  return (XFRAME (frame)->pointer_invisible ? Qnil : Qt);
+}
 
 \f
 /***********************************************************************
@@ -4623,6 +4638,7 @@ automatically.  See also `mouse-autoselect-window'.  */);
   defsubr (&Sset_frame_width);
   defsubr (&Sset_frame_size);
   defsubr (&Sset_frame_position);
+  defsubr (&Sframe_pointer_visible_p);
 
 #ifdef HAVE_WINDOW_SYSTEM
   defsubr (&Sx_get_resource);
-- 
1.7.1


[-- Attachment #3: Type: text/plain, Size: 79 bytes --]


-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Eli Zaretskii]

> From: Julien Danjou <julien@danjou.info>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 18 Oct 2010 14:12:01 +0200
> 
> >> +pointer in @var{frame}. This is useful when
> >> +@code{make-pointer-invisible} is set to @code{t}: it allows to know if
> >
> > It is a good idea to have here a cross-reference to where
> > make-pointer-invisible is described (in the Emacs User Manual).
> 
> Ah, now you want me to learn Texinfo. ;)

Just a little bit ;-)

> I've added a @xref, hoping that's what you meant.

Yes, that's what I meant.  Thanks.

Re: [PATCH 1/2] Add frame-pointer-visible-p

[Stefan Monnier]

> Fixed.

Thanks, installed (with minor changes: a few ". " converted to ".  "
and the "Returns" changed to the imperative "Return").


        Stefan

[PATCH 2/2] avoid: ignore mouse when it is hidden

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/ChangeLog |    5 +++++
 lisp/avoid.el  |    3 ++-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 674e7ff..9395ee5 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2010-10-14  Julien Danjou  <julien@danjou.info>
+
+	* avoid.el (mouse-avoidance-ignore-p): Ignore mouse when it is
+	hidden by `make-pointer-invisible'.
+
 2010-10-14  Chong Yidong  <cyd@stupidchicken.com>
 
 	* cus-face.el (custom-theme-set-faces): Call custom-push-theme
diff --git a/lisp/avoid.el b/lisp/avoid.el
index adfb1dd..4b713b8 100644
--- a/lisp/avoid.el
+++ b/lisp/avoid.el
@@ -278,7 +278,8 @@ redefine this function to suit your own tastes."
 
 (defun mouse-avoidance-ignore-p ()
   (let ((mp (mouse-position)))
-    (or executing-kbd-macro	       ; don't check inside macro
+    (or (not (frame-pointer-visible-p)) ; The pointer is hidden
+        executing-kbd-macro	       ; don't check inside macro
 	(null (cadr mp))	       ; don't move unless in an Emacs frame
 	(not (eq (car mp) (selected-frame)))
 	;; Don't do anything if last event was a mouse event.
-- 
1.7.1

Re: [PATCH 2/2] avoid: ignore mouse when it is hidden

[Stefan Monnier]

>  (defun mouse-avoidance-ignore-p ()
>    (let ((mp (mouse-position)))
> -    (or executing-kbd-macro	       ; don't check inside macro
> +    (or (not (frame-pointer-visible-p)) ; The pointer is hidden
> +        executing-kbd-macro	       ; don't check inside macro
>  	(null (cadr mp))	       ; don't move unless in an Emacs frame
>  	(not (eq (car mp) (selected-frame)))
>  	;; Don't do anything if last event was a mouse event.

Just wondering here: seeing how you've gone through the trouble to write
a patch for the C code, there must be a good reason, but why check (not
(frame-pointer-visible-p)) rather than make-pointer-invisible?


        Stefan

Re: [PATCH 2/2] avoid: ignore mouse when it is hidden

[Julien Danjou]

On Mon, Oct 18 2010, Stefan Monnier wrote:

> Just wondering here: seeing how you've gone through the trouble to write
> a patch for the C code, there must be a good reason, but why check (not
> (frame-pointer-visible-p)) rather than make-pointer-invisible?

There is a good reason! `make-pointer-invisible' is useful, but not
enough: it only makes the cursor invisible when type text.

So if your pointer is visible and you are moving your cursor in a
buffer, the pointer does not become invisible. Therefore, mouse-avoidance
can be useful in such a case.

On the opposite, if you typed text, your cursor is invisible. If you
start moving the cursor, it might encounter the pointer which is already
invisible. But since mouse-avoidance does not know it is invisible, it
makes it jump, even if it's useless.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

Re: [PATCH 2/2] avoid: ignore mouse when it is hidden

[Stefan Monnier]

>  (defun mouse-avoidance-ignore-p ()
>    (let ((mp (mouse-position)))
> -    (or executing-kbd-macro	       ; don't check inside macro
> +    (or (not (frame-pointer-visible-p)) ; The pointer is hidden
> +        executing-kbd-macro	       ; don't check inside macro
>  	(null (cadr mp))	       ; don't move unless in an Emacs frame
>  	(not (eq (car mp) (selected-frame)))
>  	;; Don't do anything if last event was a mouse event.

Installed as well,


        Stefan