Changeset View
Standalone View
part.cpp
Show First 20 Lines • Show All 293 Lines • ▼ Show 20 Line(s) | |||||
294 | } | 294 | } | ||
295 | #endif | 295 | #endif | ||
296 | 296 | | |||
297 | int Okular::Part::numberOfParts = 0; | 297 | int Okular::Part::numberOfParts = 0; | ||
298 | 298 | | |||
299 | namespace Okular | 299 | namespace Okular | ||
300 | { | 300 | { | ||
301 | 301 | | |||
302 | bool isError; | ||||
302 | Part::Part(QWidget *parentWidget, | 303 | Part::Part(QWidget *parentWidget, | ||
303 | QObject *parent, | 304 | QObject *parent, | ||
304 | const QVariantList &args) | 305 | const QVariantList &args) | ||
305 | : KParts::ReadWritePart(parent), | 306 | : KParts::ReadWritePart(parent), | ||
306 | m_tempfile( nullptr ), m_documentOpenWithPassword( false ), m_swapInsteadOfOpening( false ), m_isReloading( false ), m_fileWasRemoved( false ), m_showMenuBarAction( nullptr ), m_showFullScreenAction( nullptr ), m_actionsSearched( false ), | 307 | m_tempfile( nullptr ), m_documentOpenWithPassword( false ), m_swapInsteadOfOpening( false ), m_isReloading( false ), m_fileWasRemoved( false ), m_showMenuBarAction( nullptr ), m_showFullScreenAction( nullptr ), m_actionsSearched( false ), | ||
307 | m_cliPresentation(false), m_cliPrint(false), m_embedMode(detectEmbedMode(parentWidget, parent, args)), m_generatorGuiClient(nullptr), m_keeper( nullptr ) | 308 | m_cliPresentation(false), m_cliPrint(false), m_cliPrintAndExit(false), m_embedMode(detectEmbedMode(parentWidget, parent, args)), m_generatorGuiClient(nullptr), m_keeper( nullptr ) | ||
308 | { | 309 | { | ||
309 | // make sure that the component name is okular otherwise the XMLGUI .rc files are not found | 310 | // make sure that the component name is okular otherwise the XMLGUI .rc files are not found | ||
310 | // when this part is used in an application other than okular (e.g. unit tests) | 311 | // when this part is used in an application other than okular (e.g. unit tests) | ||
311 | setComponentName(QStringLiteral("okular"), QString()); | 312 | setComponentName(QStringLiteral("okular"), QString()); | ||
312 | 313 | | |||
313 | const QLatin1String configFileName("okularpartrc"); | 314 | const QLatin1String configFileName("okularpartrc"); | ||
314 | 315 | | |||
315 | // first, we check if a config file name has been specified | 316 | // first, we check if a config file name has been specified | ||
▲ Show 20 Lines • Show All 1324 Lines • ▼ Show 20 Line(s) | 1596 | #endif | |||
1640 | m_generatorGuiClient = factory() ? m_document->guiClient() : nullptr; | 1641 | m_generatorGuiClient = factory() ? m_document->guiClient() : nullptr; | ||
1641 | if ( m_generatorGuiClient ) | 1642 | if ( m_generatorGuiClient ) | ||
1642 | factory()->addClient( m_generatorGuiClient ); | 1643 | factory()->addClient( m_generatorGuiClient ); | ||
1643 | if ( m_cliPrint ) | 1644 | if ( m_cliPrint ) | ||
1644 | { | 1645 | { | ||
1645 | m_cliPrint = false; | 1646 | m_cliPrint = false; | ||
1646 | slotPrint(); | 1647 | slotPrint(); | ||
1647 | } | 1648 | } | ||
1649 | else if ( m_cliPrintAndExit ) | ||||
1650 | { | ||||
1651 | slotPrint(); | ||||
1652 | } | ||||
1648 | return true; | 1653 | return true; | ||
1649 | } | 1654 | } | ||
1650 | 1655 | | |||
1651 | bool Part::openUrl( const QUrl &url ) | 1656 | bool Part::openUrl( const QUrl &url ) | ||
1652 | { | 1657 | { | ||
1653 | return openUrl( url, false /* swapInsteadOfOpening */ ); | 1658 | return openUrl( url, false /* swapInsteadOfOpening */ ); | ||
1654 | } | 1659 | } | ||
1655 | 1660 | | |||
▲ Show 20 Lines • Show All 1401 Lines • ▼ Show 20 Line(s) | 3058 | { | |||
3057 | } | 3062 | } | ||
3058 | } | 3063 | } | ||
3059 | 3064 | | |||
3060 | void Part::enableStartWithPrint() | 3065 | void Part::enableStartWithPrint() | ||
3061 | { | 3066 | { | ||
3062 | m_cliPrint = true; | 3067 | m_cliPrint = true; | ||
3063 | } | 3068 | } | ||
3064 | 3069 | | |||
3070 | void Part::enableExitAfterPrint() | ||||
3071 | { | ||||
3072 | m_cliPrintAndExit = true; | ||||
3073 | } | ||||
3074 | | ||||
3065 | void Part::slotAboutBackend() | 3075 | void Part::slotAboutBackend() | ||
3066 | { | 3076 | { | ||
3067 | const KPluginMetaData data = m_document->generatorInfo(); | 3077 | const KPluginMetaData data = m_document->generatorInfo(); | ||
3068 | if (!data.isValid()) | 3078 | if (!data.isValid()) | ||
3069 | return; | 3079 | return; | ||
3070 | 3080 | | |||
3071 | KAboutData aboutData = KAboutData::fromPluginMetaData(data); | 3081 | KAboutData aboutData = KAboutData::fromPluginMetaData(data); | ||
3072 | 3082 | | |||
▲ Show 20 Lines • Show All 122 Lines • ▼ Show 20 Line(s) | 3188 | { | |||
3195 | } | 3205 | } | ||
3196 | 3206 | | |||
3197 | // Enable the Current Page option in the dialog. | 3207 | // Enable the Current Page option in the dialog. | ||
3198 | if ( m_document->pages() > 1 && currentPage() > 0 ) | 3208 | if ( m_document->pages() > 1 && currentPage() > 0 ) | ||
3199 | { | 3209 | { | ||
3200 | printDialog->setOption( QAbstractPrintDialog::PrintCurrentPage ); | 3210 | printDialog->setOption( QAbstractPrintDialog::PrintCurrentPage ); | ||
3201 | } | 3211 | } | ||
3202 | 3212 | | |||
3203 | if ( printDialog->exec() ) | 3213 | if ( printDialog->exec() ) | ||
aacid: What is the value of isError when
if ( printDialog->exec() )
returns false? | |||||
Undefined behavior - isError can either be true or false, my bad. Submitting a new patch. dileepsankhla: Undefined behavior - isError can either be true or false, my bad. Submitting a new patch. | |||||
3204 | doPrint( printer ); | 3214 | isError = doPrint( printer ); | ||
3205 | delete printDialog; | 3215 | delete printDialog; | ||
3216 | if ( isError && m_cliPrintAndExit ) | ||||
3217 | exit ( EXIT_FAILURE ); | ||||
We discussed a bit on IRC but i don't remeber which your reason for not using QCoreApplication::exit were, can you please remind me? aacid: We discussed a bit on IRC but i don't remeber which your reason for not using QCoreApplication… | |||||
I have talked about using std::exit instead of QCoreApplication::exit here : https://phabricator.kde.org/D10249#200260 With that, I want to add that QCoreApplication::exit does nothing if the event loop is not running. You can find the info in the docs here: http://doc.qt.io/qt-5/qcoreapplication.html#exit dileepsankhla: I have talked about using std::exit instead of QCoreApplication::exit here : https… | |||||
3218 | else if ( m_cliPrintAndExit ) | ||||
3219 | exit ( EXIT_SUCCESS ); | ||||
3206 | } | 3220 | } | ||
aacid: You have far too many exit calls, i'm 99% sure you could have just one. | |||||
Agree that only one exit call is required but what about the exit status - EXIT_SUCCESS or EXIT_FAILURE? The issue description talked about the command line batch processing. For that purpose, should I set a bool "isError" when failing condition is encountered like "printing is not allowed" and then based on the isError's status, should I call either exit (EXIT_SUCCESS) or exit (EXIT_FAILURE)? dileepsankhla: Agree that only one exit call is required but what about the exit status - EXIT_SUCCESS or… | |||||
3207 | } | 3221 | } | ||
3208 | 3222 | | |||
3209 | | ||||
3210 | void Part::setupPrint( QPrinter &printer ) | 3223 | void Part::setupPrint( QPrinter &printer ) | ||
3211 | { | 3224 | { | ||
3212 | printer.setOrientation(m_document->orientation()); | 3225 | printer.setOrientation(m_document->orientation()); | ||
3213 | 3226 | | |||
3214 | // title | 3227 | // title | ||
3215 | QString title = m_document->metaData( QStringLiteral("DocumentTitle") ).toString(); | 3228 | QString title = m_document->metaData( QStringLiteral("DocumentTitle") ).toString(); | ||
3216 | if ( title.isEmpty() ) | 3229 | if ( title.isEmpty() ) | ||
3217 | { | 3230 | { | ||
3218 | title = m_document->currentDocument().fileName(); | 3231 | title = m_document->currentDocument().fileName(); | ||
3219 | } | 3232 | } | ||
3220 | if ( !title.isEmpty() ) | 3233 | if ( !title.isEmpty() ) | ||
3221 | { | 3234 | { | ||
3222 | printer.setDocName( title ); | 3235 | printer.setDocName( title ); | ||
3223 | } | 3236 | } | ||
3224 | } | 3237 | } | ||
3225 | 3238 | | |||
3226 | 3239 | | |||
3227 | void Part::doPrint(QPrinter &printer) | 3240 | bool Part::doPrint(QPrinter &printer) | ||
Can you please make doPrint return true on success and not on error? It's a much saner API. aacid: Can you please make doPrint return true on success and not on error?
It's a much saner API. | |||||
3228 | { | 3241 | { | ||
3229 | if (!m_document->isAllowed(Okular::AllowPrint)) | 3242 | if (!m_document->isAllowed(Okular::AllowPrint)) | ||
3230 | { | 3243 | { | ||
3231 | KMessageBox::error(widget(), i18n("Printing this document is not allowed.")); | 3244 | KMessageBox::error(widget(), i18n("Printing this document is not allowed.")); | ||
3232 | return; | 3245 | return true; | ||
3233 | } | 3246 | } | ||
3234 | 3247 | | |||
3235 | if (!m_document->print(printer)) | 3248 | if (!m_document->print(printer)) | ||
3236 | { | 3249 | { | ||
3237 | const QString error = m_document->printError(); | 3250 | const QString error = m_document->printError(); | ||
3238 | if (error.isEmpty()) | 3251 | if (error.isEmpty()) | ||
3239 | { | 3252 | { | ||
3240 | KMessageBox::error(widget(), i18n("Could not print the document. Unknown error. Please report to bugs.kde.org")); | 3253 | KMessageBox::error(widget(), i18n("Could not print the document. Unknown error. Please report to bugs.kde.org")); | ||
3241 | } | 3254 | } | ||
3242 | else | 3255 | else | ||
3243 | { | 3256 | { | ||
3244 | KMessageBox::error(widget(), i18n("Could not print the document. Detailed error is \"%1\". Please report to bugs.kde.org", error)); | 3257 | KMessageBox::error(widget(), i18n("Could not print the document. Detailed error is \"%1\". Please report to bugs.kde.org", error)); | ||
3245 | } | 3258 | } | ||
3259 | return true; | ||||
3246 | } | 3260 | } | ||
3261 | return false; | ||||
3247 | } | 3262 | } | ||
3248 | 3263 | | |||
3249 | void Part::psTransformEnded(int exit, QProcess::ExitStatus status) | 3264 | void Part::psTransformEnded(int exit, QProcess::ExitStatus status) | ||
3250 | { | 3265 | { | ||
3251 | Q_UNUSED( exit ) | 3266 | Q_UNUSED( exit ) | ||
3252 | if ( status != QProcess::NormalExit ) | 3267 | if ( status != QProcess::NormalExit ) | ||
3253 | return; | 3268 | return; | ||
3254 | 3269 | | |||
▲ Show 20 Lines • Show All 231 Lines • ▼ Show 20 Line(s) | |||||
3486 | { | 3501 | { | ||
3487 | const KPluginMetaData data = m_document->generatorInfo(); | 3502 | const KPluginMetaData data = m_document->generatorInfo(); | ||
3488 | m_aboutBackend->setEnabled(data.isValid()); | 3503 | m_aboutBackend->setEnabled(data.isValid()); | ||
3489 | } | 3504 | } | ||
3490 | 3505 | | |||
3491 | void Part::resetStartArguments() | 3506 | void Part::resetStartArguments() | ||
3492 | { | 3507 | { | ||
3493 | m_cliPrint = false; | 3508 | m_cliPrint = false; | ||
3509 | m_cliPrintAndExit = false; | ||||
3494 | } | 3510 | } | ||
3495 | 3511 | | |||
3496 | #if PURPOSE_FOUND | 3512 | #if PURPOSE_FOUND | ||
3497 | void Part::slotShareActionFinished(const QJsonObject &output, int error, const QString &message) | 3513 | void Part::slotShareActionFinished(const QJsonObject &output, int error, const QString &message) | ||
3498 | { | 3514 | { | ||
3499 | if (error) { | 3515 | if (error) { | ||
3500 | KMessageBox::error(widget(), i18n("There was a problem sharing the document: %1", message), | 3516 | KMessageBox::error(widget(), i18n("There was a problem sharing the document: %1", message), | ||
3501 | i18n("Share")); | 3517 | i18n("Share")); | ||
Show All 24 Lines |
What is the value of isError when
returns false?