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

[Eli Zaretskii <eliz@gnu.org>]

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