[Merge] lp:~jonas-drange/ubuntu-ui-extras/consolidated-devices into lp:ubuntu-ui-extras/staging
Andrew Hayzen
andrew.hayzen at canonical.com
Tue Apr 4 10:28:14 UTC 2017
Review: Needs Fixing
Looks good so far, just 3 inline comments.
1) It seems like this should be set?
Otherwise further down this if will be false
if (makeLower == "hp") {
...
}
2) Same here
3) Should this then do
makeLower = make.toLower();
Diff comments:
>
> === added file 'modules/Ubuntu/Components/Extras/Printers/cups/ppdutils.cpp'
> --- modules/Ubuntu/Components/Extras/Printers/cups/ppdutils.cpp 1970-01-01 00:00:00 +0000
> +++ modules/Ubuntu/Components/Extras/Printers/cups/ppdutils.cpp 2017-03-31 18:01:46 +0000
> @@ -0,0 +1,344 @@
> +/*
> + * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2014, 2015 Red Hat, Inc.
> + * Copyright (C) 2006 Florian Festi <ffesti at redhat.com>
> + * Copyright (C) 2006, 2007, 2008, 2009 Tim Waugh <twaugh at redhat.com>
> + * Copyright (C) 2017 Canonical, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "ppdutils.h"
> +
> +#include <QSet>
> +
> +const QRegularExpression PpdUtils::versionNumbers = QRegularExpression(
> + " v(?:er\\.)?\\d(?:\\d*\\.\\d+)?(?: |$)"
> +);
> +const QRegularExpression PpdUtils::ignoredSuffixes = QRegularExpression(
> + ","
> + "| hpijs"
> + "| foomatic/"
> + "| - "
> + "| w/"
> + "| \\("
> + "| postscript"
> + "| ps"
> + "| ps3"
> + "| pdf"
> + "| pxl"
> + "| zjs"
> + "| zxs"
> + "| pcl3"
> + "| printer"
> + "|_bt"
> + "| pcl"
> + "| ufr ii"
> + "| br-script"
> +);
> +const QRegularExpression PpdUtils::ignoreSeries = QRegularExpression(
> + " series| all-in-one",
> + QRegularExpression::CaseInsensitiveOption
> +);
> +const QList<QPair<QString, QRegularExpression>> PpdUtils::manufacturerByModels
> + = QList<QPair<QString, QRegularExpression>>({
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("HP"), QRegularExpression(
> + "deskjet"
> + "|dj[ 0-9]?"
> + "|laserjet"
> + "|lj"
> + "|color laserjet"
> + "|color lj"
> + "|designjet"
> + "|officejet"
> + "|oj"
> + "|photosmart"
> + "|ps "
> + "|psc"
> + "|edgeline"
> + )
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Epson"), QRegularExpression("stylus|aculaser")
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Apple"), QRegularExpression(
> + "stylewriter"
> + "|imagewriter"
> + "|deskwriter"
> + "|laserwriter"
> + )
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Canon"), QRegularExpression(
> + "pixus"
> + "|pixma"
> + "|selphy"
> + "|imagerunner"
> + "|bj"
> + "|lbp"
> + )
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Brother"), QRegularExpression("hl|dcp|mfc")
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Xerox"), QRegularExpression(
> + "docuprint"
> + "|docupage"
> + "|phaser"
> + "|workcentre"
> + "|homecentre"
> + )
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Lexmark"),
> + QRegularExpression("optra|(:color )?jetprinter")
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("KONICA MINOLTA"), QRegularExpression(
> + "magicolor"
> + "|pageworks"
> + "|pagepro"
> + )
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Kyocera"), QRegularExpression(
> + "fs-"
> + "|km-"
> + "|taskalfa"
> + )
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Ricoh"), QRegularExpression("aficio")
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Oce"), QRegularExpression("varioprint")
> + ),
> + QPair<QString, QRegularExpression>(
> + QStringLiteral("Oki"), QRegularExpression("okipage|microline")
> + ),
> +});
> +
> +const QMap<QString, QString> PpdUtils::hpByModel = QMap<QString, QString>{
> + {"dj", "DeskJet"},
> + {"lj", "LaserJet"},
> + {"laserjet", "LaserJet"},
> + {"oj", "OfficeJet"},
> + {"officejet", "OfficeJet"},
> + {"color lj", "Color LaserJet"},
> + {"ps ", "PhotoSmart"},
> + {"hp ", ""},
> +};
> +
> +QString PpdUtils::dedupeMakeModel(const QString &str)
> +{
> + QSet<QString> stringSet;
> + QStringList deduped;
> + Q_FOREACH(auto m, str.split(" ")) {
> + if (!stringSet.contains(m)) {
> + deduped << m;
> + }
> + stringSet << m;
> + }
> + return deduped.join(" ");
> +}
> +
> +QPair<QString, QString> PpdUtils::splitMakeModel(const QString &makeModel)
> +{
> + const QString mm = dedupeMakeModel(makeModel);
> + const QString makeModelLower = mm.toLower();
> +
> + QString make;
> + QString model;
> +
> + /* Informs us whether or not the make part was produced in such a way that
> + it might need cleaning. I.e. we've made a guess at what the make is. */
> + bool cleanupMake = false;
> +
> + /* If the string starts with a known model name (like "LaserJet") assume
> + that the manufacturer name is missing and add the manufacturer name
> + corresponding to the model name */
> + Q_FOREACH(auto manufacturerByModel, PpdUtils::manufacturerByModels) {
> + if (manufacturerByModel.second.match(makeModelLower).hasMatch()) {
> + make = manufacturerByModel.first;
> + model = mm;
> + }
> + }
> +
> + /* Take the first word as the name of the manufacturer.
> +
> + Note that we have skipped a bunch of special cases where the first word
> + isn't necessarily the manufacturer, hence the next TODO.
> +
> + TODO: Handle special cases for TurboPrint, lexmark international,
> + fuji xerox, etc. */
> + int firstSpace = mm.indexOf(" ");
> + if (make.isEmpty() && firstSpace > 0) {
> + make = mm.left(firstSpace);
> + model = mm.mid(make.size() + 1);
> +
> + /* We've picked the first part as the make, but it may be that we've
> + split "hewlett packard" and are left with "hewlett". */
> + cleanupMake = true;
> + }
> +
> + // Standardised names for manufacturers.
> + const QString makeLower = make.toLower();
> + if (cleanupMake) {
> + if (makeLower.startsWith("hewlett") && makeLower.endsWith("packard")) {
> + make = "HP";
> + // makeLower = "hp"
It seems like this should be set?
Otherwise further down this if will be false
if (makeLower == "hp") {
...
}
> + } else if (makeLower.startsWith("konica") &&
> + makeLower.endsWith("minolta")) {
> + make = "KONICA MINOLTA";
> + // makeLower = "konica minolta"
Same here
> + } else {
> + // Fix case errors.
> + Q_FOREACH(auto manufacturerByModel, manufacturerByModels) {
> + if (makeLower == manufacturerByModel.first.toLower()) {
> + make = manufacturerByModel.first;
Should this then do
makeLower = make.toLower();
> + break;
> + }
> + }
> + }
> + }
> +
> + // Remove the word "Series" if present.
> + model = model.remove(ignoreSeries);
> +
> + /* Find a version number and cut the model from there. Note that some
> + models may legitimately look like version numbers. So we cut only if the
> + version number has max one digit, or a dot with digits before and after. */
> + QString modelLower = model.toLower();
> + int versionAt = modelLower.indexOf(" v");
> + if (versionAt >= 0) {
> + // Look for v or ver. followed by digits and dots.
> + auto match = versionNumbers.match(modelLower);
> + if (match.hasMatch()) {
> + int matchAt = match.capturedStart();
> + model.truncate(matchAt);
> + modelLower.truncate(matchAt);
> + }
> + }
> +
> + // Remove ignored suffixes.
> + auto suffixMatch = ignoredSuffixes.match(modelLower);
> + if (suffixMatch.hasMatch()) {
> + int matchAt = suffixMatch.capturedStart();
> + model.truncate(matchAt);
> + modelLower.truncate(matchAt);
> + }
> +
> + // Replace lj or dj with LaserJet and DeskJet respectively, etc.
> + if (makeLower == "hp") {
> + Q_FOREACH(auto name, hpByModel.keys()) {
> + if (modelLower.contains(name)) {
> + int start = modelLower.indexOf(name);
> + model.remove(start, name.size());
> + model.insert(start, hpByModel[name]);
> + modelLower = model.toLower();
> + }
> + }
> + }
> +
> + return QPair<QString, QString>(make, model);
> +}
> +
> +QString PpdUtils::normalize(const QString &str)
> +{
> + QString strLower = str.trimmed().toLower();
> + QString normalized;
> +
> + const int BLANK = 0;
> + const int ALPHA = 1;
> + const int DIGIT = 2;
> + int lastchar = BLANK;
> +
> + bool alnumfound = false;
> +
> + for (int i = 0; i < strLower.size(); i++) {
> + if (strLower[i].isLetter()) {
> + if (lastchar != ALPHA && alnumfound) {
> + normalized += " ";
> + }
> + lastchar = ALPHA;
> + } else if (strLower[i].isDigit()) {
> + if (lastchar != DIGIT && alnumfound) {
> + normalized += " ";
> + }
> + lastchar = DIGIT;
> + } else {
> + lastchar = BLANK;
> + }
> +
> + if (strLower[i].isLetterOrNumber()) {
> + normalized += strLower[i];
> + alnumfound = true;
> + }
> + }
> + return normalized;
> +}
> +
> +PrinterEnum::DuplexMode PpdUtils::ppdChoiceToDuplexMode(const QString &choice)
> +{
> + if (choice == "DuplexTumble")
> + return PrinterEnum::DuplexMode::DuplexShortSide;
> + else if (choice == "DuplexNoTumble")
> + return PrinterEnum::DuplexMode::DuplexLongSide;
> + else
> + return PrinterEnum::DuplexMode::DuplexNone;
> +}
> +
> +const QString PpdUtils::duplexModeToPpdChoice(
> + const PrinterEnum::DuplexMode &mode)
> +{
> + switch (mode) {
> + case PrinterEnum::DuplexMode::DuplexShortSide:
> + return "DuplexTumble";
> + case PrinterEnum::DuplexMode::DuplexLongSide:
> + return "DuplexNoTumble";
> + case PrinterEnum::DuplexMode::DuplexNone:
> + default:
> + return "None";
> + }
> +}
> +
> +
> +ColorModel PpdUtils::parsePpdColorModel(const QString &name,
> + const QString &text,
> + const QString &optionName)
> +{
> + ColorModel ret;
> + ret.name = name;
> + ret.text = text;
> + ret.originalOption = optionName;
> +
> + if (ret.name.contains("Gray") || ret.name.contains("Black")) {
> + ret.colorType = PrinterEnum::ColorModelType::GrayType;
> + } else {
> + ret.colorType = PrinterEnum::ColorModelType::ColorType;
> + }
> + return ret;
> +}
> +
> +PrintQuality PpdUtils::parsePpdPrintQuality(const QString &choice,
> + const QString &text,
> + const QString &optionName)
> +{
> + PrintQuality quality;
> + quality.name = choice;
> + quality.text = text;
> + quality.originalOption = optionName;
> + return quality;
> +}
--
https://code.launchpad.net/~jonas-drange/ubuntu-ui-extras/consolidated-devices/+merge/321497
Your team Ubuntu Phablet Team is subscribed to branch lp:ubuntu-ui-extras/staging.
More information about the Ubuntu-reviews
mailing list