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