bug#51658: [PATCH] Haiku port (again)

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: 51658@debbugs.gnu.org
> Date: Sun, 14 Nov 2021 09:08:17 +0800
> 
> >> +  HAVE_M17N_FLT=no
> >> +  if test "${HAVE_LIBOTF}" = yes; then
> >> +    if test "${with_m17n_flt}" != "no"; then
> >> +      EMACS_CHECK_MODULES([M17N_FLT], [m17n-flt])
> >> +      if test "$HAVE_M17N_FLT" = "yes"; then
> >> +  AC_DEFINE(HAVE_M17N_FLT, 1, [Define to 1 if using libm17n-flt.])
> >> +      fi
> >> +    fi
> >> +  fi
> 
> > Do we need libm17n-flt?  For which font back-end is it needed?
> 
> The Cairo font backend has the ability to use libm17n-flt.  The code for
> that is in ftcrfont.c, as it is on X-Windows.

Let's use Cairo with HarfBuzz instead, then.  I don't want to use
libm17n-flt in any new code: it is unmaintained and has known
problems.

> > This sounds no different from any other system, so I wonder why this
> > needs to be described.  On most systems, there's no Super or Hyper
> > keys, and they can be simulated via "C-x @", unless the user
> > configures the keyboard to do some remapping.
> 
> The "option key" and "command key" in Haiku are unusual concepts not
> found in other systems, so it's worthwhile to mention it here.

But the users of Haiku know about those keys, don't they?  The Emacs
manual is not supposed to teach the users about their systems, it's
supposed to teach them about Emacs.

> >> +  On Haiku, Emacs defaults to using the system tooltip mechanism.
> >> +This usually leads to more responsive tooltips, but the tooltips will
> >> +not be able to use advanced display features.  If that is what you
> >> +need, you can disable the use of system tooltips by setting the
> >> +variable @code{haiku-use-system-tooltips} to nil. (@pxref{Tooltips,,,
> >> +elisp, The Emacs Lisp Reference Manual}).
> 
> > How is this different from the GTK build?  If it isn't different,
> > maybe it shouldn't be mentioned at all.
> 
> The variable used is different, as it doesn't make sense to name a Haiku
> variable `x-gtk-...'

But you already describe the variable elsewhere, so why does it have
to be mentioned in this appendix as well?

> >> +@table @key
> >> +@item haiku-refs-found
> >> +This event is received whenever the operating system tells Emacs to
> >> +open a file, such as when a file's icon is dropped onto the
> >> +@code{Emacs} binary in the Tracker, or when a file is dropped onto a
> >> +frame.  The event itself is a list, the second item of which is a
> >> +string containing the path of the file to be opened.
> 
> > Why isn't this treated as drag-and-drop on other platforms?  Then you
> > won't need Haiku-specific documentation and events.
> 
> It might not specifically be a drag event: for example, the Tracker
> could ask Emacs to open a file, because the user selected it from the
> "Recent files" menu.

The result is the same: Emacs visits a file.  I don't think I
understand why these events should be exposed to Lisp.

> >> +@item haiku-quit-requested
> >> +This event is received whenever the operating system tells Emacs that
> >> +the user has asked Emacs to quit, optionally seeking confirmation if
> >> +the user has any unsaved documents. By default, it is bound to
> >> +@code{save-buffers-kill-emacs}, see @ref{Exiting}.
> 
> > And this should be connected to the session manager, see xsmfns.c, and
> > then it can just trigger the mechanism defined there, as on other
> > platforms.
> 
> Isn't the mechanism there specific to X?

Which part is specific to X?

And if the current implementation uses X-specific code, it just means
the implementation should be extended to allow other platforms trigger
the same mechanism.  Any reason Haiku couldn't do that?  Having
disparate platform-specific mechanisms for performing the same task is
bad for maintenance and bad for documentation and users.  We should
try to present as uniform UI as possible on all platforms, even when
the internal details are different.

> >> @@ -2191,7 +2199,8 @@ Management Parameters
> >>  @code{mouse-autoselect-window} (@pxref{Mouse Window Auto-selection}).
> >>  This may have the unwanted side-effect that a user cannot scroll a
> >>  non-selected frame with the mouse.  Some window managers may not honor
> >> -this parameter.
> >> +this parameter.  On Haiku, it also has the side-effect that the window
> >> +will not be able to receive any keyboard input from the user.
> 
> > I don't understand: non-selected windows don't receive keyboard input
> > on all systems.  So what is special about Haiku here?
> 
> `no-accept-focus' means that the frame isn't willing to accept focus
> from mouse clicks and other mouse motion events.
> 
> On X-Windows, for instance, it is possible to give the keyboard focus to
> such a frame by hitting Alt-TAB (in most window managers); this is not
> possible on Haiku.

Then the text should explicitly mention Alt-TAB.

> > There isn't a single comment in this and other Haiku-specific source
> > files.  Please consider adding at least comments describing what the
> > important non-trivial functions do and what their arguments mean.
> 
> Many of these functions are described in the header file defining their
> prototypes, haiku_support.h.

I realize that many projects document functions in header files, but
we don't.  Our style is to document them in the *.c files instead.

> >> +/* Haiku doesn't expose font language data in BFont objects.  Thus, we
> >> +   select a few representative characters for each supported `:lang'
> >> +   (currently Chinese, Korean and Japanese,) and test for those
> >> +   instead.  */
> >> +
> >> +static uint32_t language_code_points[MAX_LANGUAGE][4] =
> >> +  {{20154, 20754, 22996, 0}, /* Chinese.  */
> >> +   {51312, 49440, 44544, 0}, /* Korean.  */
> >> +   {26085, 26412, 12371, 0}, /* Japanese.  */};
> 
> AFAIK, `script-representative-chars' doesn't handle languages such as
> Korean or Japanese, but I could be mistaken.

Of course, it does.  It just goes by script, not by language.  But if
script-representative-chars lacks some characters you need, we could
add them as alternatives.

> >> +_Noreturn void
> >> +gui_abort (const char *msg)
> >> +{
> >> +  fprintf (stderr, "Abort in GUI code: %s\n", msg);
> >> +  fprintf (stderr, "Under Haiku, Emacs cannot recover from errors in GUI 
> >> code\n");
> >> +  fprintf (stderr, "App Server disconnects usually manifest as bitmap "
> >> +     "initialization failures or lock failures.");
> >> +  emacs_abort ();
> >> +}
> 
> > Are these writes to stderr going to stay in the production code?
> 
> It's used to report an error in Emacs that will cause it to abort
> immediately afterwards, with some explanation as to why.  It will be
> valuable for bug reports, as `addr2line', `gdb' and friends are hard to
> set up on Haiku, and the output of the system debugger is not very
> helpful with crashes in Emacs C++ code.  So I think it's OK.  (Something
> similar is done in xterm to explain the GTK display disconnect abort
> bug.)

Is it guaranteed that Emacs has stderr stream connected to some file
or device when it runs in GUI session?  If not, this stuff will be
lost, and it might be a better idea to pop up a GUI dialog window with
this text instead.

> >> +Lisp_Object
> >> +list_system_processes (void)
> >> +{
> 
> > Can the Haiku-specific bits here be moved to some Haiku-specific file?
> 
> It would be more consistent with the other surrounding Unix variants
> (i.e. GNU/Linux, Darwin, and the BSDs) to define them in that file.
> 
> But if you still insist, I have no objection it it.

I'd prefer to have it elsewhere.  We already have quite a mes with
this in sysdep.c.

> Thanks, please see the revised patch.

Will review this later.