bug#70989: 29.3; Calling isearch-forward when using Unifont throws error "Invalid font name"

[Eli Zaretskii <eliz@gnu.org>]

> From: Po Lu <luangruo@yahoo.com>
> Cc: me@rodrigomorales.site,  70989@debbugs.gnu.org
> Date: Sun, 19 May 2024 19:13:32 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Po Lu <luangruo@yahoo.com>
> >> Cc: me@rodrigomorales.site,  70989@debbugs.gnu.org
> >> Date: Sun, 19 May 2024 15:37:09 +0800
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> > What I see there is the call to fond_intern_prop, and if the result is
> >> > accepted by font_style_to_value as a valid value of FONT_WIDTH_INDEX,
> >> > it is returned.  But "Sans-Serif" is not supposed to be a valid width
> >> > of a font, so I'm confused.
> >> 
> >> Where does FONT_WIDTH_INDEX come into play?
> >
> > Inside get_adstyle_property:
> >
> >   adstyle = font_intern_prop (str, end - str, 1);
> >   if (font_style_to_value (FONT_WIDTH_INDEX, adstyle, 0) >= 0)
> >     return Qnil;
> >   return adstyle;
> 
> This tests whether the adstyle duplicates information that the pattern
> might provide as the font width, since if so, the adstyle is entirely
> redundant.  You'll observe that font_style_to_value returning >= 0
> indicates that the adstyle is a valid width, and therefore the function
> rejects, and does not validate, width values.

Thanks.

> >> The attribute responsible is FONT_ADSTYLE_INDEX, which is set to
> >> "Sans-Serif" by:
> >> 
> >> 	 font. */
> >>       FT_Face ft_face;
> >> 
> >>       ASET (entity, FONT_ADSTYLE_INDEX, get_adstyle_property (p));
> >>       if ((ft_library || FT_Init_FreeType (&ft_library) == 0)
> >> 
> >> in ftfont_pattern_entity.  The XLFD generated from a font with such an
> >> adstyle is not parsable, the character `-' being the field separator in
> >> XLFDs, and gives rise to parser errors such as Rodrigo encountered when
> >> I-search, by proxy, attempted to deconstruct the XLFD back into a font
> >> spec.
> >
> > Yes, I understand all that.  That was not my question.
> >
> > So what is the patch you'd propose.
> 
> diff --git a/src/ftfont.c b/src/ftfont.c
> index 2e37b62ea35..efb6bf668bb 100644
> --- a/src/ftfont.c
> +++ b/src/ftfont.c
> @@ -149,7 +149,8 @@ matching_prefix (char const *str, ptrdiff_t len, char const *pat)
>  get_adstyle_property (FcPattern *p)
>  {
>    FcChar8 *fcstr;
> -  char *str, *end;
> +  char *str, *end, *tmp;
> +  size_t i;
>    Lisp_Object adstyle;
>  
>  #ifdef FC_FONTFORMAT
> @@ -168,7 +169,18 @@ get_adstyle_property (FcPattern *p)
>        || matching_prefix (str, end - str, "Oblique")
>        || matching_prefix (str, end - str, "Italic"))
>      return Qnil;
> -  adstyle = font_intern_prop (str, end - str, 1);
> +  /* The characters `-', `?', `*', and `"' are not representable in XLFDs
> +     and therefore must be replaced by substitutes.  (bug#70989) */
> +  USE_SAFE_ALLOCA;
> +  tmp = SAFE_ALLOCA (end - str);
> +  for (i = 0; i < end - str; ++i)
> +    tmp[i] = (str[i] == '-'
> +	      ? '_' : ((str[i] != '?'
> +			&& str[i] != '*'
> +			&& str[i] != '"')
> +		       ? str[i] : ' '));
> +  adstyle = font_intern_prop (tmp, end - str, 1);
> +  SAFE_FREE ();
>    if (font_style_to_value (FONT_WIDTH_INDEX, adstyle, 0) >= 0)
>      return Qnil;
>    return adstyle;

Thanks, but why not replace all of those with a SPC?  It's simpler, I
think.