Changeset View
Changeset View
Standalone View
Standalone View
common/utils.cpp
Show All 27 Lines | |||||
28 | 28 | | |||
29 | QString Utils::outputName(const KScreen::OutputPtr& output) | 29 | QString Utils::outputName(const KScreen::OutputPtr& output) | ||
30 | { | 30 | { | ||
31 | return outputName(output.data()); | 31 | return outputName(output.data()); | ||
32 | } | 32 | } | ||
33 | 33 | | |||
34 | QString Utils::outputName(const KScreen::Output *output) | 34 | QString Utils::outputName(const KScreen::Output *output) | ||
35 | { | 35 | { | ||
36 | const auto concatWithSpace = [](const QString ¤t, const QString &toAppend) -> QString { | ||||
romangg: Please don't use single character variable names. Also can this be made more explicit? I can't… | |||||
37 | const auto toAppendTrimmed = toAppend.trimmed(); | ||||
38 | // If we already have some text in "current", append a space and the new text. | ||||
39 | if (!current.isEmpty() && !toAppendTrimmed.isEmpty()) { | ||||
40 | return current + QLatin1Char(' ') + toAppendTrimmed; | ||||
41 | // If we already have some text, but there's nothing to append, return the original text. | ||||
42 | } else if (!current.isEmpty() && toAppendTrimmed.isEmpty()) { | ||||
43 | return current; | ||||
44 | } | ||||
45 | | ||||
46 | // Otherwise return just the new text (might be an empty string, but that's OK) | ||||
romangg: full-stop | |||||
47 | return toAppendTrimmed; | ||||
48 | }; | ||||
49 | | ||||
36 | if (output->type() == KScreen::Output::Panel) { | 50 | if (output->type() == KScreen::Output::Panel) { | ||
37 | return i18n("Laptop Screen"); | 51 | return i18n("Laptop Screen"); | ||
38 | } | 52 | } | ||
39 | 53 | | |||
40 | if (output->edid()) { | 54 | if (output->edid()) { | ||
41 | // The name will be "VendorName ModelName (ConnectorName)", | 55 | // The name will be "VendorName ModelName (ConnectorName)", | ||
42 | // but some components may be empty. | 56 | // but some components may be empty. | ||
43 | QString name; | 57 | QString name; | ||
44 | if (!(output->edid()->vendor().isEmpty())) { | 58 | name = concatWithSpace(name, output->edid()->vendor()); | ||
45 | name = output->edid()->vendor() + QLatin1Char(' '); | 59 | name = concatWithSpace(name, output->edid()->name()); | ||
46 | } | 60 | | ||
47 | if (!output->edid()->name().isEmpty()) { | 61 | // Don't append (ConnectorName) if its the same as "VendorName ModelName". | ||
48 | name += output->edid()->name() + QLatin1Char(' '); | 62 | if (!name.isEmpty() && name != output->name()) { | ||
49 | } | 63 | return concatWithSpace(name, QLatin1Char('(') + output->name() + QLatin1Char(')')); | ||
romangg: Full-stop in the end. | |||||
50 | if (!name.trimmed().isEmpty()) { | | |||
51 | return name + QLatin1Char('(') + output->name() + QLatin1Char(')'); | | |||
52 | } | 64 | } | ||
If I am reading this correctly, you could keep the old code and use simplified(): then later on: return name + QLatin1Char('(') + outName + QLatin1Char(')'); } ahmadsamir: If I am reading this correctly, you could keep the old code and use simplified():
const QString… | |||||
Not really, the catch is in the whitespace at the end of name (then "Vendor Model " == "Vendor Model" is never true) dvratil: Not really, the catch is in the whitespace at the end of `name` (then `"Vendor Model " ==… | |||||
53 | } | 65 | } | ||
54 | return output->name(); | 66 | return output->name(); | ||
55 | } | 67 | } | ||
56 | 68 | | |||
57 | QString Utils::sizeToString(const QSize &size) | 69 | QString Utils::sizeToString(const QSize &size) | ||
58 | { | 70 | { | ||
59 | return QStringLiteral("%1x%2").arg(size.width()).arg(size.height()); | 71 | return QStringLiteral("%1x%2").arg(size.width()).arg(size.height()); | ||
60 | } | 72 | } | ||
61 | 73 | |
Please don't use single character variable names. Also can this be made more explicit? I can't guess directly what the meaning of a, b and s is. I don't think it is that generic, is it?