CUPS: Rework set/clearCupsOption API

Previously it was a bit awkward to use, you had to do
    QStringList cupsOptions = QCUPSSupport::cupsOptionsList(printer);
    QCUPSSupport::setCupsOption(cupsOptions, option, value);
    QCUPSSupport::setCupsOptions(printer, cupsOptions);
now you simply have to do
    QCUPSSupport::setCupsOption(printer, option, value);

Change-Id: Id31583f1ec72644791d82776debbae5583a2be54
Reviewed-by: Andy Shaw <andy.shaw@qt.io>

3 files changed, 20 insertions, 34 deletions

diff --git a/src/printsupport/dialogs/qprintdialog_unix.cpp b/src/printsupport/dialogs/qprintdialog_unix.cpp
index 47e2fa58f0..ab38c240e3 100644
--- a/src/printsupport/dialogs/qprintdialog_unix.cpp
+++ b/src/printsupport/dialogs/qprintdialog_unix.cpp
@@ -1417,9 +1417,7 @@ void QPPDOptionsModel::setCupsOptionsFromItems(QPrinter *printer, QOptionTreeIte
             QOptionTreeItemOption *itmOption = static_cast<QOptionTreeItemOption *>(itm);
             const ppd_option_t *opt = static_cast<const ppd_option_t*>(itm->ptr);
             if (qstrcmp(opt->defchoice, opt->choices[itmOption->selected].choice) != 0) {
-                QStringList cupsOptions = QCUPSSupport::cupsOptionsList(printer);
-                QCUPSSupport::setCupsOption(cupsOptions, QString::fromLatin1(opt->keyword), QString::fromLatin1(opt->choices[itmOption->selected].choice));
-                QCUPSSupport::setCupsOptions(printer, cupsOptions);
+                QCUPSSupport::setCupsOption(printer, QString::fromLatin1(opt->keyword), QString::fromLatin1(opt->choices[itmOption->selected].choice));
             }
         } else {
             setCupsOptionsFromItems(printer, itm);
diff --git a/src/printsupport/kernel/qcups.cpp b/src/printsupport/kernel/qcups.cpp
index 115ecb0ac2..be170ed619 100644
--- a/src/printsupport/kernel/qcups.cpp
+++ b/src/printsupport/kernel/qcups.cpp
@@ -43,33 +43,37 @@
 
 QT_BEGIN_NAMESPACE
 
-QStringList QCUPSSupport::cupsOptionsList(QPrinter *printer)
+static QStringList cupsOptionsList(QPrinter *printer) Q_DECL_NOTHROW
 {
     return printer->printEngine()->property(PPK_CupsOptions).toStringList();
 }
 
-void QCUPSSupport::setCupsOptions(QPrinter *printer, const QStringList &cupsOptions)
+void setCupsOptions(QPrinter *printer, const QStringList &cupsOptions) Q_DECL_NOTHROW
 {
     printer->printEngine()->setProperty(PPK_CupsOptions, QVariant(cupsOptions));
 }
 
-void QCUPSSupport::setCupsOption(QStringList &cupsOptions, const QString &option, const QString &value)
+void QCUPSSupport::setCupsOption(QPrinter *printer, const QString &option, const QString &value)
 {
+    QStringList cupsOptions = cupsOptionsList(printer);
     if (cupsOptions.contains(option)) {
         cupsOptions.replace(cupsOptions.indexOf(option) + 1, value);
     } else {
         cupsOptions.append(option);
         cupsOptions.append(value);
     }
+    setCupsOptions(printer, cupsOptions);
 }
 
-void QCUPSSupport::clearCupsOption(QStringList &cupsOptions, const QString &option)
+void QCUPSSupport::clearCupsOption(QPrinter *printer, const QString &option)
 {
+    QStringList cupsOptions = cupsOptionsList(printer);
     // ### use const_iterator once QList::erase takes them
     const QStringList::iterator it = std::find(cupsOptions.begin(), cupsOptions.end(), option);
     if (it != cupsOptions.end()) {
         Q_ASSERT(it + 1 < cupsOptions.end());
         cupsOptions.erase(it, it+1);
+        setCupsOptions(printer, cupsOptions);
     }
 }
 
@@ -140,30 +144,24 @@ QCUPSSupport::JobHoldUntilWithTime QCUPSSupport::parseJobHoldUntil(const QString
 
 void QCUPSSupport::setJobHold(QPrinter *printer, const JobHoldUntil jobHold, const QTime &holdUntilTime)
 {
-    QStringList cupsOptions = cupsOptionsList(printer);
     const QString jobHoldUntilArgument = jobHoldToString(jobHold, holdUntilTime);
     if (!jobHoldUntilArgument.isEmpty()) {
-        setCupsOption(cupsOptions,
+        setCupsOption(printer,
                       QStringLiteral("job-hold-until"),
                       jobHoldUntilArgument);
     } else {
-        clearCupsOption(cupsOptions, QStringLiteral("job-hold-until"));
+        clearCupsOption(printer, QStringLiteral("job-hold-until"));
     }
-    setCupsOptions(printer, cupsOptions);
 }
 
 void QCUPSSupport::setJobBilling(QPrinter *printer, const QString &jobBilling)
 {
-    QStringList cupsOptions = cupsOptionsList(printer);
-    setCupsOption(cupsOptions, QStringLiteral("job-billing"), jobBilling);
-    setCupsOptions(printer, cupsOptions);
+    setCupsOption(printer, QStringLiteral("job-billing"), jobBilling);
 }
 
 void QCUPSSupport::setJobPriority(QPrinter *printer, int priority)
 {
-    QStringList cupsOptions = cupsOptionsList(printer);
-    setCupsOption(cupsOptions, QStringLiteral("job-priority"), QString::number(priority));
-    setCupsOptions(printer, cupsOptions);
+    setCupsOption(printer, QStringLiteral("job-priority"), QString::number(priority));
 }
 
 static inline QString bannerPageToString(const QCUPSSupport::BannerPage bannerPage)
@@ -209,17 +207,14 @@ QCUPSSupport::JobSheets QCUPSSupport::parseJobSheets(const QString &jobSheets)
 
 void QCUPSSupport::setBannerPages(QPrinter *printer, const BannerPage startBannerPage, const BannerPage endBannerPage)
 {
-    QStringList cupsOptions = cupsOptionsList(printer);
     const QString startBanner = bannerPageToString(startBannerPage);
     const QString endBanner   = bannerPageToString(endBannerPage);
 
-    setCupsOption(cupsOptions, QStringLiteral("job-sheets"), startBanner + QLatin1Char(',') + endBanner);
-    setCupsOptions(printer, cupsOptions);
+    setCupsOption(printer, QStringLiteral("job-sheets"), startBanner + QLatin1Char(',') + endBanner);
 }
 
 void QCUPSSupport::setPageSet(QPrinter *printer, const PageSet pageSet)
 {
-    QStringList cupsOptions = cupsOptionsList(printer);
     QString pageSetString;
 
     switch (pageSet) {
@@ -234,22 +229,19 @@ void QCUPSSupport::setPageSet(QPrinter *printer, const PageSet pageSet)
         break;
     }
 
-    setCupsOption(cupsOptions, QStringLiteral("page-set"), pageSetString);
-    setCupsOptions(printer, cupsOptions);
+    setCupsOption(printer, QStringLiteral("page-set"), pageSetString);
 }
 
 void QCUPSSupport::setPagesPerSheetLayout(QPrinter *printer,  const PagesPerSheet pagesPerSheet,
                                           const PagesPerSheetLayout pagesPerSheetLayout)
 {
-    QStringList cupsOptions = cupsOptionsList(printer);
     // WARNING: the following trick (with a [2]-extent) only works as
     // WARNING: long as there's only one two-digit number in the list
     // WARNING: and it is the last one (before the "\0")!
     static const char pagesPerSheetData[][2] = { "1", "2", "4", "6", "9", {'1', '6'}, "\0" };
     static const char pageLayoutData[][5] = {"lrtb", "lrbt", "rlbt", "rltb", "btlr", "btrl", "tblr", "tbrl"};
-    setCupsOption(cupsOptions, QStringLiteral("number-up"), QLatin1String(pagesPerSheetData[pagesPerSheet]));
-    setCupsOption(cupsOptions, QStringLiteral("number-up-layout"), QLatin1String(pageLayoutData[pagesPerSheetLayout]));
-    setCupsOptions(printer, cupsOptions);
+    setCupsOption(printer, QStringLiteral("number-up"), QLatin1String(pagesPerSheetData[pagesPerSheet]));
+    setCupsOption(printer, QStringLiteral("number-up-layout"), QLatin1String(pageLayoutData[pagesPerSheetLayout]));
 }
 
 void QCUPSSupport::setPageRange(QPrinter *printer, int pageFrom, int pageTo)
@@ -259,9 +251,7 @@ void QCUPSSupport::setPageRange(QPrinter *printer, int pageFrom, int pageTo)
 
 void QCUPSSupport::setPageRange(QPrinter *printer, const QString &pageRange)
 {
-    QStringList cupsOptions = cupsOptionsList(printer);
-    setCupsOption(cupsOptions, QStringLiteral("page-ranges"), pageRange);
-    setCupsOptions(printer, cupsOptions);
+    setCupsOption(printer, QStringLiteral("page-ranges"), pageRange);
 }
 
 QT_END_NAMESPACE
diff --git a/src/printsupport/kernel/qcups_p.h b/src/printsupport/kernel/qcups_p.h
index a5af4bcbb0..0afa7fa12f 100644
--- a/src/printsupport/kernel/qcups_p.h
+++ b/src/printsupport/kernel/qcups_p.h
@@ -129,10 +129,8 @@ public:
         TopToBottomRightToLeft
     };
 
-    static QStringList cupsOptionsList(QPrinter *printer);
-    static void setCupsOptions(QPrinter *printer, const QStringList &cupsOptions);
-    static void setCupsOption(QStringList &cupsOptions, const QString &option, const QString &value);
-    static void clearCupsOption(QStringList &cupsOptions, const QString &option);
+    static void setCupsOption(QPrinter *printer, const QString &option, const QString &value);
+    static void clearCupsOption(QPrinter *printer, const QString &option);
 
     static void setJobHold(QPrinter *printer, const JobHoldUntil jobHold = NoHold, const QTime &holdUntilTime = QTime());
     static void setJobBilling(QPrinter *printer, const QString &jobBilling = QString());