Differential D28122 Diff 78124 src/kfontchooserdialog.cpp

Changeset View

Standalone View

View Options

src/kfontchooserdialog.cpp

  • This file was added.
1​/*
2​Copyright (C) 1996 Bernd Johannes Wuebben <wuebben@kde.org>
3​Copyright (c) 1999 Preston Brown <pbrown@kde.org>
4​Copyright (c) 1999 Mario Weilguni <mweilguni@kde.org>
5​
6​This library is free software; you can redistribute it and/or
7​modify it under the terms of the GNU Library General Public
8​License as published by the Free Software Foundation; either
9​version 2 of the License, or (at your option) any later version.
10​
11​This library is distributed in the hope that it will be useful,
12​but WITHOUT ANY WARRANTY; without even the implied warranty of
13​MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14​Library General Public License for more details.
15​
16​You should have received a copy of the GNU Library General Public License
17​along with this library; see the file COPYING.LIB. If not, write to
18​the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
19​Boston, MA 02110-1301, USA.
20​*/
21​
22​#include "kfontchooserdialog.h"
23​
24​#include <QDialogButtonBox>
25​#include <QPushButton>
26​#include <QVBoxLayout>
27​
28​class KFontChooserDialogPrivate
29​{
30​public:
31​ KFontChooserDialogPrivate()
32​ : chooser(nullptr)
33​ {
34​ }
35​
36​ KFontChooser *chooser;
37​
38​ void stripRegularStyleName(QFont &font);
39​};
dfaureUnsubmitted
Done

Should be static. Even file-static (move the implementation further up in the file).

dfaure: Should be static. Even file-static (move the implementation further up in the file).
40​
41​KFontChooserDialog::KFontChooserDialog(QWidget *parent, const KFontChooser::DisplayFlags &flags, const QStringList &fontList)
42​ : QDialog(parent),
43​ d(new KFontChooserDialogPrivate)
44​{
45​ setWindowTitle(tr("Select Font"));
46​ d->chooser = new KFontChooser(this, flags, fontList, 8, nullptr);
47​ d->chooser->setObjectName(QStringLiteral("fontChooser"));
48​
49​ connect(d->chooser, &KFontChooser::fontSelected, this, &KFontChooserDialog::fontSelected);
50​
51​ QDialogButtonBox *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
52​ QVBoxLayout *mainLayout = new QVBoxLayout;
53​ setLayout(mainLayout);
54​ mainLayout->addWidget(d->chooser);
55​ mainLayout->addWidget(buttonBox);
56​
57​ connect(buttonBox, &QDialogButtonBox::accepted, this, &QDialog::accept);
58​ connect(buttonBox, &QDialogButtonBox::rejected, this, &QDialog::reject);
59​}
60​
61​KFontChooserDialog::~KFontChooserDialog()
62​{
63​ delete d;
64​}
65​
66​void KFontChooserDialog::setFont(const QFont &font, bool onlyFixed)
67​{
68​ d->chooser->setFont(font, onlyFixed);
69​}
70​
71​QFont KFontChooserDialog::font() const
72​{
73​ return d->chooser->font();
74​}
75​
76​// static
77​int KFontChooserDialog::getFontDiff(QFont &theFont, KFontChooser::FontDiffFlags &diffFlags,
78​ const KFontChooser::DisplayFlags &flags, QWidget *parent)
79​{
80​ KFontChooserDialog dlg(parent, flags | KFontChooser::ShowDifferences, QStringList());
81​ dlg.setModal(true);
82​ dlg.setObjectName(QStringLiteral("Font Selector"));
dfaureUnsubmitted
Done

not needed, exec() makes it modal anyway

dfaure: not needed, exec() makes it modal anyway
83​ dlg.setFont(theFont, flags & KFontChooser::FixedFontsOnly);
84​
85​ int result = dlg.exec();
86​ if (result == Accepted) {
dfaureUnsubmitted
Done

const

dfaure: const
87​ theFont = dlg.d->chooser->font();
88​ diffFlags = dlg.d->chooser->fontDiffFlags();
89​ dlg.d->stripRegularStyleName(theFont);
90​ }
91​ return result;
92​}
93​
94​// static
mevenUnsubmitted
Not Done

Shouldn't this be not commented

meven: Shouldn't this be not commented
ahmadsamirAuthorUnsubmitted
Not Done

IIUC, if a function is declared static inside the class body (e.g. in the header file), the static keyword can't be repeated when it's defined outside the class body.

ahmadsamir: IIUC, if a function is declared static inside the class body (e.g. in the header file), the…
95​int KFontChooserDialog::getFont(QFont &theFont, const KFontChooser::DisplayFlags &flags, QWidget *parent)
96​{
97​ KFontChooserDialog dlg(parent, flags, QStringList());
98​ dlg.setModal(true);
kossebauUnsubmitted
Not Done

Is having dialog objects on the stack a good idea?

What if the application gets closed while the dialog is open, would that not crash?

At least at some point people thought it was not a good idea, see https://blogs.kde.org/node/3919

kossebau: Is having dialog objects on the stack a good idea? What if the application gets closed while…
ahmadsamirAuthorUnsubmitted
Not Done

hmmm..... dfaure knows more about exec() and event loops more than me. @dfaure WDYT?

ahmadsamir: hmmm..... dfaure knows more about exec() and event loops more than me. @dfaure WDYT?
dfaureUnsubmitted
Not Done

The theoretical answer is yes, this would crash. But note that the user cannot just close the application by clicking somewhere while a modal dialog is up. This requires much more subtle interaction like a DBus call.

There are a million modal dialogs out there created on the stack, but sure, if you want to be pedantic, use QPointer, new (and manual delete, for lack of a proper smart pointer for this)...

(or we could set WA_DeleteOnClose instead of the manual delete? I just added a comment to the blog about that)

dfaure: The theoretical answer is yes, this would crash. But note that the user cannot just close the…
ahmadsamirAuthorUnsubmitted
Not Done

OK, given how many modal dialogs are out there, I'll leave it as is.

ahmadsamir: OK, given how many modal dialogs are out there, I'll leave it as is.
99​ dlg.setObjectName(QStringLiteral("Font Selector"));
dfaureUnsubmitted
Done

same

dfaure: same
100​ dlg.setFont(theFont, flags & KFontChooser::FixedFontsOnly);
101​
102​ int result = dlg.exec();
103​ if (result == Accepted) {
104​ theFont = dlg.d->chooser->font();
105​ dlg.d->stripRegularStyleName(theFont);
106​ }
107​ return result;
108​}
109​
110​// If the styleName property is set for a QFont, using setBold(true) would
111​// lead to Qt using an "emboldended"/synthetic font style instead of using
mevenUnsubmitted
Not Done

Shouldn't this be not commented

meven: Shouldn't this be not commented
112​// the bold style provided by the font itself; the latter looks better than
113​// the former, also accorgin to upstream, the styleName property is useful
114​// for fancy font styles, so there is no point in setting it for "Regular"
115​// fonts. For more details see:
116​// https://bugreports.qt.io/browse/QTBUG-63792
117​// https://bugs.kde.org/show_bug.cgi?id=378523
118​void KFontChooserDialogPrivate::stripRegularStyleName(QFont &font)
119​{
120​ if (font.weight() == QFont::Normal
121​ && (font.styleName() == QLatin1String("Regular")
122​ || font.styleName() == QLatin1String("Normal")
123​ || font.styleName() == QLatin1String("Book")
124​ || font.styleName() == QLatin1String("Roman"))) {
125​ font.setStyleName(QString());
126​ }
127​}