Draft: Rewrite of the patch for Arabic font

Speaking as upstream: while this patch isn't upstreamable as such, upstream still has an interest in distro patches being the highest quality and correctness they can be. As such, I've taken the liberty to review this proposed patch. Hope that helps!

Yes it helps, @chpe, much appreciated!

Speaking as upstream: while this patch isn't upstreamable as such

Does upstream acknowledge the issue? The Arabic speaking Ubuntu user, who raised the issue, tested several monospace fonts including Noto Sans Mono, and found that of the fonts he tested only DejaVu Sans Mono is free from the rendering issue in VTE based terminals. What about e.g. Source Code Pro? (Probably he didn't test that.)

So this patch makes these locales use the Monospace font.

This also used to be the default monospace font in gsettings-desktop-schemas, but was (for "reasons") changed a few years ago. Since Monospace is also the default font in the gnome-terminal schemas, I'd be inclined to make use-system-font default to false in the g-t profile prefs, so g-t will use its own pref value instead of the desktop-wide font. That should then fix this, unless the user explicitly selects a different font, in which case that's on them.

So this patch makes these locales use the Monospace font.

The first version of it did, but I switched to set DejaVu Sans Mono explicitly.

This also used to be the default monospace font in gsettings-desktop-schemas, but was (for "reasons") changed a few years ago.

Yeah, upstream has Source Code Pro, Debian has Monospace, and Ubuntu has Ubuntu Mono. It's a mess.

Since Monospace is also the default font in the gnome-terminal schemas, I'd be inclined to make use-system-font default to false in the g-t profile prefs, so g-t will use its own pref value instead of the desktop-wide font. That should then fix this, unless the user explicitly selects a different font, in which case that's on them.

That would indeed make a difference, but with the obvious drawback that g-t would be detached by default from the monospace preferences set in Tweaks, including the font size. The rendering issue for Arabic would also remain in some cases:

If I understand it correctly, Monospace is not the name of a font but a generic term which makes it query fontconfig about which monospace font to pick. And the Arabic rendering issue makes it necessary to link the language to the used font.

Nowadays with fontconfig 2.14 Noto Sans Mono will be picked for most languages in Debian and sometimes in Ubuntu:

$ LC_CTYPE=en_US.UTF-8 fc-match monospace
NotoSansMono-Regular.ttf: "Noto Sans Mono" "Regular"

It looks like fontconfig will keep picking DejaVu Sans Mono for Arabic locales, though:

$ LC_CTYPE=ar_EG.UTF-8 fc-match monospace
DejaVuSansMono.ttf: "DejaVu Sans Mono" "Book"

So in cases where the user for instance has set some Arabic language via the LANGUAGE or LC_MESSAGES variable, while LC_CTYPE is something else, Monospace may result in the rendering issue being present.

So even if I'm biased (apparently), I think there are reasons to special case Arabic in the code as proposed.

P.S. If gnome-terminal is replaced with gnome-console as default terminal, there will be no own pref value to rely on.

added 2 commits

500bfe1a - Fix memory leak
df674298 - Use g_get_language_names() and enforce DejaVu Sans Mono

Compare with previous version

changed the description

added 1 commit

bad25b32 - Use pango to change family

Compare with previous version

added 1 commit

559158bb - Use auto keyword and rename patch

Compare with previous version

mentioned in commit 16b3db1b

The proposed changed in this MR were committed manually, so closing.

closed

Draft: Rewrite of the patch for Arabic font

Merge request reports

Activity