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

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

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

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.

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

Thanks, fixed.

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

The "option key" and "command key" in Haiku are unusual concepts not
found in other systems, so it's worthwhile to mention it here.

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

Okay, thanks.

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

Done, thanks.

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

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

Thanks.

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

Good catch, thanks.

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

>> +@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?  I could not find an example of
any other port using it, and it seems to use X specific functions.

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

Makes sense, I deleted it, thanks.

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

Thanks.

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

Thanks.

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

Understood, that entry seems to be entirely redundant, so it has been
deleted.

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

Thanks, I did that.

> This sounds like stuff for PROBLEMS again.

Thanks.

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

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

Thanks, I replaced it with a reference.

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

Thanks, all done.

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

Without ucs-normalize, some mysterious error occured during fontset
creation earlier in development, but it seems to be gone now, so I
deleted it along with the form that loaded `international/mule-util',
thanks.

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

Thanks, I tried.

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

USE_CAIRO, when defined, tries to define a lot of X related features
that aren't necessary.  USE_BE_CAIRO only enables the parts required for
the Cairo font driver to operate correctly.

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

Many of these functions are described in the header file defining their
prototypes, haiku_support.h.  The ones that are not documented (such as
most of the drawing functions) are ones that behave identically to the
Haiku counterparts they wrap (i.e. BView_FillTriangle), and ones that
behave identically to the documentation of Haiku functions by almost the
same name, but reimplemented to work around bugs in Haiku
(i.e. BView_DrawMask).

I may have missed a few of the function that do need documentation
though.  I fixed some of that in the revised patch.

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

I tried my best to audit those comments and delete the ones that did not
make sense.  Many of them are valid, because I have tried to keep the
code as close as possible to the X and w32 code.

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

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

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

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

The call to perror is redundant: gui_abort should suffice.  So they have
been deleted.

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

Thanks, fixed.

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

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.

> Thanks.

Thanks, please see the revised patch.

haiku-emacs.patch
Description: Text Data