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

From: Eli Zaretskii <[email-masked]>
Date: Sat, 13 Nov 2021 20:31:12 +0200

> From: Po Lu <[email-masked]>
> Cc: [email-masked]
> Date: Thu, 11 Nov 2021 15:40:01 +0800
> 
> Here is the updated commit message; the patch is attached.

See the comments below.

> +  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?

> +@cindex TTY Emacs in haiku

@cindex entries should begin from a lower-case letter, so that they
sort predictably regardless of the locale where the manual is built.

> +  If you are launching Emacs from the Tracker, or want to make the
> +Tracker open files using Emacs, you should use the binary named
> +@code{Emacs}; If you are going to use Emacs in the terminal, or wish
                 ^^
Should be "if", not capitalized.

> +to launch separate instances of Emacs, or do not care for the
> +aforementioned system integration features, the binary named

What "system integration features" does this text refer to?  It's
unclear from the text.

> +@code{emacs} should be used instead.
                ^^^^^^^^^^^^^^
Passive tense!

> +  On Haiku, unusual modifier keys such as the Hyper key are
> +unsupported.  By default, the super key corresponds with the option
> +key defined by the operating system, the meta key with the command
> +key, the control key with the system control key, and the shift key
> +with the system shift key.  On a standard PC keyboard, Haiku should
> +map these keys to positions familiar to those using a GNU system, but
> +this may require some adjustment to your system's configuration to
> +work.

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.

> +  If the system shift key is held down, Emacs will always use the
> +system shift keymap regardless of what other modifier keys are
> +currently held down.  Otherwise, if any other modifier key is held
> +down, the default system keymap will be used to map keys.  This makes
> +it impossible to type accented characters using the system super key
> +map.

This description focuses on the underlying technical issue, and leaves
the user-facing behavior not sufficiently clear.  This is a user
manual, so it is best to describe the behavior users will see, and
leave out the technical details.  It sounds like all you want to say
is that accented letters cannot be typed using the super key? then
just say that without going int the details too much.

> +@cindex haiku input methods
> +@cindex BeCJK and Emacs
> +  Some popular Haiku input methods such BeCJK are known to behave
> +badly when interacting with Emacs.  If you are affected, you can use
> +an Emacs input method instead.  See @ref{Input Methods}.

This should be in PROBLEMS, not here.

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

> +@subsection What to do when Emacs crashes
> +@cindex haiku debugger
> +@vindex haiku-debug-on-fatal-error

This should have an index entry starting from "crashes", so that users
could find it easily.

> +@node Haiku Customization
> +@section Customizations specific to Haiku
> +  This section details several customizations that are specific to
> +Haiku windowing.
> +
> +@table @code
> +@vindex haiku-use-system-tooltips
> +@item haiku-use-system-tooltips
> +This variable controls whether or not system tooltips will be used.
> +
> +When non-nil, tooltips are displayed by the Application Kit and not
> +Emacs, and accordingly do not have a tooltip frame.  As such, they are
> +also unable to utilize any display properties.
> +
> +@vindex haiku-use-system-debugger
> +@item haiku-use-system-debugger
> +When non-nil, Emacs will ask the system to launch the system debugger
> +whenever it experiences a fatal error.  This behaviour is standard
> +among Haiku applications.
> +@end table

These descriptions should be in the same place where the corresponding
behaviors are described.  Having them in a separate section makes them
harder to discover.

> +@subsection Modifier keys
> +@cindex modifier key customization (Haiku)
> +You can customize which Emacs modifiers the various system modifier
> +keys correspond to through the following variables:
> +
> +@table @code
> +@vindex haiku-meta-keysym
> +@item haiku-meta-keysym
> +The system modifier key that will be treated as the Meta key by Emacs.
> +It defaults to @code{command}.
> +
> +@vindex haiku-control-keysym
> +@item haiku-control-keysym
> +The system modifier key that will be treated as the Control key by
> +Emacs.  It defaults to @code{control}.
> +
> +@vindex haiku-super-keysym
> +@item haiku-super-keysym
> +The system modifier key that will be treated as the Super key by
> +Emacs.  It defaults to @code{option}.

These should be in the same section that describes the keyboard
peculiarities.

> +@vindex haiku-shift-keysym
> +@item haiku-super-keysym
               ^^^^^
Copy/paste error?

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

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

> +@node Haiku Resources
> +@section Using the X defaults database on Haiku
> +@cindex X resources, haiku
> +@cindex x-get-resource, haiku
> +
> +  As there is no equivalent to an X resource database on Haiku, a
> +facsimile is provided in its place.  This facsimile does not attempt
> +to process X resource files, but is responsive to the @kbd{--xrm}
> +command-line argument.  For details about using this argument, see
> +@ref{Resources}.

"Resources" is a general section, unrelated to Haiku.  If there's
nothing special we must say about resources on Haiku, why do we need
this section?

> +@node Haiku Fonts
> +@section Font and font backend selection on Haiku
> +@cindex font backend selection (Haiku)

This should be reworded to say that Haiku uses 2 font backends used by
other platforms, and has one Haiku-specific backend.  There's no
reason to describe the backends that are named the same as their
GNU/Linux counterparts, except if their behavior significantly
differs.

> +  The @code{ftcr} font backend is available whenever Emacs is built
> +with Cairo support.  It is the most fully featured, but will cause
> +display issues when used on a frame with double-buffering disabled.

The right place for this is in PROBLEMS again.

> +  This font backend has a known problem with displaying certain bold
> +or italic fonts on older Haiku systems.  It is documented in
> +@file{etc/PROBLEMS}, see @ref{Known Problems}.

No need to repeat this in the manual.

> +  The @code{ftcr} backend features HarfBuzz variants when Emacs is
               ^^^^
Typo (but I see no reason to have this part in the manual, see above).

> @@ -8250,6 +8254,12 @@ Tooltips
>  @code{nil}.  The rest of this subsection describes how to control
>  non-GTK+ tooltips, which are presented by Emacs itself.
>  
> +@vindex haiku-use-system-tooltips
> +When Emacs is built with the Haiku Application Kit, it usually displays
> +tooltips using Application Kit functions.  When using these tooltips, text
> +properties inside tooltip text will be unavailable. To disable these
> +tooltips, change the value of @code{haiku-use-system-tooltips} to @code{nil}.

It is not a good idea to have the same feature described in more than
one place: it leads to inconsistencies, and is generally a maintenance
burden.  Decide whether you want to describe this here or in the Haiku
appendix, and either have the description in that one place, or leave
only the cross-reference in the other with minimum text.  And make
sure the index entry leads to the place with the actual description,
not to the place where you have a cross-reference.

> +On Haiku, setting @code{fullscreen} to @code{fullwidth} or
> +@code{fullheight} has no effect.

This is not important enough to be in the text; please make it a
@footnote.

> +If Emacs is built with Haiku and Cairo, inhibiting double buffering on
> +a frame may lead to severe artifacting when display elements such as
> +the mouse cursor pass over a frame.

This sounds like stuff for PROBLEMS again.

> @@ -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?

> @@ -2352,7 +2361,13 @@ Font and Color Parameters
>  engine), and @code{harfbuzz} (font driver for OTF and TTF fonts with
>  HarfBuzz text shaping) (@pxref{Windows Fonts,,, emacs, The GNU Emacs
>  Manual}).  The @code{harfbuzz} driver is similarly recommended.  On
> -other systems, there is only one available font backend, so it does
> +Haiku, there are up to two font drivers: @code{haiku} (the server-side
> +font driver, always available), and @code{ftcr} (a font-driver using
> +Cairo, only available if Emacs was built with Cairo).  For
> +@code{ftcr}, there also exists a variant that shapes text using
> +HarfBuzz, named @code{ftcrhb}.

This is an unnecessary repetition of what you already say in the
appendix.

> +Lowering frames is not supported on Haiku, due to limitations imposed
> +by the system.

This should also be in a @footnote.

> +Restacking frames is not supported on Haiku, due to limitations
> +imposed by the system.

And this.

> +  On Haiku, child frames are only visible when a parent frame is
> +active, owing to a limitation of the Haiku windowing system.  Owing to
> +the same limitation, child frames are only guaranteed to appear above
> +their top-level parent; that is to say, the top-most frame in the
> +hierarchy, which does not have a parent frame.

And this.

> +** Emacs has been ported to the Haiku operating system.
> +The configuration process should automatically detect and build for Haiku.
> +There is also an optional window-system port to Haiku, which can be enabled
> +by configuring Emacs with the option '--with-be-app', which will require
> +the Haiku Application Kit development headers and a C++ compiler to be present
> +on your system.  If Emacs is not built with the option '--with-be-app', the
> +resulting Emacs will only run in text-mode terminals.
> +
> ++++
> +** Cairo drawing support has been enabled for Haiku builds.
> +To enable Cairo support, ensure that the Cairo and FreeType development files
> +are present on your system, and configure Emacs with '--with-be-cairo'.
> +
> +---
> +** Double buffering is now enabled on the Haiku operating system.
> +Unlike X, there is no compile-time option to enable or disable double-buffering.
> +If you wish to disable double-buffering, change the frame parameter
> +`inhibit-double-buffering' instead.
> +

This should be a single "**" parent entry with "***" sub-entries.

> --- a/lisp/loadup.el
> +++ b/lisp/loadup.el
> @@ -302,6 +302,13 @@
>        (load "term/common-win")
>        (load "term/x-win")))
>  
> +(if (featurep 'haiku)
> +    (progn
> +      (load "term/common-win")
> +      (load "term/haiku-win")
> +      (load "international/mule-util")
> +      (load "international/ucs-normalize")))

Why do you need ucs-normalize?  And what about the condition not to
load it if uni-*.el files are unavailable?

> --- a/src/floatfns.c
> +++ b/src/floatfns.c
> @@ -347,6 +347,19 @@ DEFUN ("logb", Flogb, Slogb, 1, 1, 0,
>  double_integer_scale (double d)
>  {
>    int exponent = ilogb (d);
> +#ifdef HAIKU
> +  /* On Haiku, the values of FP_ILOGB0 and FP_ILOGBNAN are identical.
> +     To top it all, both are also identical to INT_MAX.  */
> +  if (exponent == FP_ILOGB0)
> +    {
> +      if (isnan (d))
> +	return (DBL_MANT_DIG - DBL_MIN_EXP) + 2;
> +      if (isinf (d))
> +	return (DBL_MANT_DIG - DBL_MIN_EXP) + 1;
> +
> +      return (DBL_MANT_DIG - DBL_MIN_EXP);
> +    }
> +#endif

I don't understand what the situation with FP_ILOGB0 has to do with
logb function's implementation and the workaround you add here.
Please clarify the comment.

> -#ifdef USE_CAIRO
> +#if defined (USE_CAIRO) || defined (USE_BE_CAIRO)

Does this mean that USE_CAIRO code is not used by Haiku when Emacs is
built with Cairo on Haiku?  Why is that?  And why cannot Haiku use
USE_CAIRO?

> diff --git a/src/haiku_draw_support.cc b/src/haiku_draw_support.cc
> new file mode 100644
> index 0000000000..c04bed81c2
> --- /dev/null
> +++ b/src/haiku_draw_support.cc

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.

Btw, many comments in source files that are similar to X and w32 seem
to have been copied wholesale from those other systems.  I have no way
of verifying whether the copied comments are accurate for Haiku, so
it's up to you to audit them.

> +/* 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.  */};

Why not use script-representative-chars here?

> +_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?

> +	key = strdup (ky);
> +	if (!key)
> +	  {
> +	    perror ("strdup");
> +	    gui_abort ("strdup failed");
> +	  }
> +      }
> +
> +    if (help)
> +      {
> +	this->help = strdup (help);
> +	if (!this->help)
> +	  {
> +	    perror ("strdup");
> +	    gui_abort ("strdup failed");
> +	  }
> +      }

And these calls to perror?

> -#ifdef HAVE_EXT_MENU_BAR
> +#if defined (HAVE_EXT_MENU_BAR) || defined (HAVE_HAIKU)

Why is this needed?  You did define HAVE_EXT_MENU_BAR on Haiku, right?

> @@ -3119,6 +3143,23 @@ list_system_processes (void)
>    return  proclist;
>  }
>  
> +#elif defined HAIKU
> +
> +Lisp_Object
> +list_system_processes (void)
> +{

Can the Haiku-specific bits here be moved to some Haiku-specific file?

Thanks.