Commit 3e49c920 authored by Harald Sitter's avatar Harald Sitter 🏳️‍🌈
Browse files

kbd preview: don't fall over unfortunate arg combos

as it turns out it's fairly easy to fail the original xkb assert when
using variants with certain models that don't actually sport support for
the variants

e.g. `--model applealu_iso --layout gb --variant mac_intl`

to deal with this we no longer assume that geometry compilation will
actually work and instead switch the qml ui into an error state when
problems appear. furthermore we'll run the arg combination through
setxkbmap|xkbcomp to get a sense of why the server might have failed to
compile the geometry and use that as detailed error output

I need to point out that the preview failing doesn't necessarily mean
the layout application as a whole will because there's different code
paths the kcm and the kded will take to apply layouts so depending on
which path is taken the configuration may be partially applied or not at
all (e.g. with the scenario from above the end result may be that model
doesn't get applied but the layout and variant will)

since these changes required some rejiggering of the pointers and their
life times this has also seen some related cleanup. notably the geometry
is now in charge of the life time of the root xkb object and the
previous qsharedpointer cleanup (which was really scoped pointer cleanup
but scoped pointer has kinda sad api for custom deleters) is now instead
a unique_ptr with custom deleter.
parent 69a0efcd
......@@ -43,3 +43,8 @@ Geometry::Geometry(XkbGeometryPtr geom_, XkbDescPtr xkb_, QObject *parent)
sections.push_back(new Section(geom->sections + i, xkb, this));
XkbFreeKeyboard(xkb, 0, True);
......@@ -32,6 +32,7 @@ class Geometry : public XkbObject
Geometry(XkbGeometryPtr geom_, XkbDescPtr xkb_, QObject *parent = nullptr);
~Geometry() override;
XkbGeometryPtr geom = nullptr;
......@@ -23,10 +23,13 @@
#include <QQmlApplicationEngine>
#include <QQmlContext>
#include <QX11Info>
#include <QProcess>
#include <KAboutData>
#include <KLocalizedString>
#include <memory>
#include "application.h"
#include "config-workspace.h"
#include "doodad.h"
......@@ -106,16 +109,19 @@ int main(int argc, char *argv[])
QSharedPointer<XkbRF_RulesRec> rulesCleanup(rules, [](XkbRF_RulesPtr obj) {
std::unique_ptr<XkbRF_RulesRec, std::function<void(XkbRF_RulesPtr)>> rulesCleanup(rules, [](XkbRF_RulesPtr obj) {
XkbRF_Free(obj, True);
XkbComponentNamesRec componentNames;
memset(&componentNames, 0, sizeof(XkbComponentNamesRec));
XkbRF_GetComponents(rules, &varDefs, &componentNames);
QString errorDetails;
std::unique_ptr<Geometry> geometry;
XkbDescPtr xkb = XkbGetKeyboardByName(QX11Info::display(),
......@@ -126,12 +132,22 @@ int main(int argc, char *argv[])
XkbGBN_ClientSymbolsMask |
QSharedPointer<XkbDescRec> xkbCleanup(xkb, [](XkbDescPtr obj) {
XkbFreeKeyboard(obj, 0, True);
Geometry geometry(xkb->geom, xkb);
if (!xkb) {
QProcess setxkbmap;
QProcess xkbcomp;
xkbcomp.setProcessChannelMode(QProcess::MergedChannels); // combine in single channel
setxkbmap.start(QStringLiteral("setxkbmap"), {QStringLiteral("-print"), QStringLiteral("-model"), model, QStringLiteral("-layout"), layout, QStringLiteral("-variant"), variant, QStringLiteral("-option"), options});
xkbcomp.start(QStringLiteral("xkbcomp"), {QStringLiteral("-")});
errorDetails = xkbcomp.readAllStandardOutput();
} else {
geometry.reset(new Geometry(xkb->geom, xkb));
// Register the doodads so we can perform easy type checks with them
// and determine how to render the individual object.
......@@ -155,8 +171,11 @@ int main(int argc, char *argv[])
if (!obj && url == objUrl)
}, Qt::QueuedConnection);
engine.rootContext()->setContextProperty("geometry", &geometry);
engine.rootContext()->setContextProperty("geometry", geometry.get());
engine.rootContext()->setContextProperty("errorDetails", errorDetails);
return app.exec();
#include "main.moc"
......@@ -21,6 +21,7 @@
import QtQuick 2.12
import QtQuick.Window 2.12
import QtQuick.Controls 2.5
import QtQuick.Layouts 1.15
import org.kde.tastenbrett.private 1.0 as XKB
......@@ -56,6 +57,19 @@ Window {
color: activePalette.window
ColumnLayout {
id: errorLayout
visible: false
anchors.fill: parent
TextArea {
Layout.fillWidth: true
text: errorDetails
readOnly: true
color: activePalette.text
Item {
id: kbd
width: window.width
......@@ -184,6 +198,12 @@ Window {
Component.onCompleted: {
if (errorDetails != "") {
errorLayout.visible = true
visible = true
// Based on the geometry aspect we scale either width or height to scale so
// the default size is fitting. NB: geom dimensions are mm!
if (geometry.widthMM >= geometry.heightMM) {
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment