Differential D15122 Diff 41173 src/widgets/atcoreinstancewidget.cpp

Changeset View

Standalone View

View Options

src/widgets/atcoreinstancewidget.cpp

Show First 20 Lines • Show All 305 Lines • ▼ Show 20 Line(s)299 connect(m_sdWidget, &SdWidget::deleteSdFile, this, [this](const QString & fileName) {
306 } else { 306​ } else {
307 m_core.sdDelete(fileName); 307​ m_core.sdDelete(fileName);
308 } 308​ }
309 }); 309​ });
310} 310​}
311 311​
312void AtCoreInstanceWidget::printFile(const QUrl &fileName) 312​void AtCoreInstanceWidget::printFile(const QUrl &fileName)
313{ 313​{
314 if (!fileName.isEmpty() && (m_core.state() == AtCore::IDLE)) { 314​ if (fileName.isEmpty()) {
315​ QMessageBox::critical(
rizzitelloUnsubmitted
Not Done

This is good but the wording of the dialog is not very descriptive of what the error was or why it happened.

I would use something more inline with
File not found for the title.

Then for the body

"The chosen file can not be printed because it no longer exists"

Something like that that is more descriptive of what the error is . And im sure we can come up with even better wording then that too.

rizzitello: This is good but the wording of the dialog is not very descriptive of what the error was or why…
316​ this
317​ , i18n("Filename Empty")
318​ , i18n("No filename sent from calling method, please check and try again."));
319​ return;
320​ } else if (!QFileInfo(fileName.toString()).isReadable()) {
321​ QMessageBox::critical(
322​ this
323​ , i18n("File not found")
324​ , i18n("%1 \nIs not readable, please check and try again.", fileName.toString()));
325​ return;
326​ }
327​ if (m_core.state() == AtCore::IDLE) {
rizzitelloUnsubmitted
Not Done

Its a shame QFileInfo does not accept a QUrl :(

rizzitello: Its a shame QFileInfo does not accept a QUrl :(
patrickelectricUnsubmitted
Not Done

also !QFileInfo(fileName)::isReadable()

patrickelectric: also !QFileInfo(fileName)::isReadable()
laysrodriguesAuthorUnsubmitted
Not Done

isReadable() cannot receive a file as param. The alternative is to create a QFileInfo obj and do the tests from it. https://doc.qt.io/qt-5/qfileinfo.html#isReadable

QFileInfo file(fileName.toString());
    if (fileName.isEmpty() || !file.exists() || file.isReadable()) {
laysrodrigues: isReadable() cannot receive a file as param. The alternative is to create a QFileInfo obj and…
rizzitelloUnsubmitted
Not Done

Good point @patrickelectric , If the file does not exist is is also not readable ? if so we should only have to check if QFileInfo::isReadable() to know if its both exsiting and readable.

rizzitello: Good point @patrickelectric , If the file does not exist is is also not readable ? if so we…
rizzitelloUnsubmitted
Not Done

Im all for doing something like this where we show a different message for either error. QFileInfo.isReadable() works well as a single check to see if it exists also.

if ( fileName.isEmpty() ) {
    QMessageBox::critical(
            this
            , i18n("Filename Empty")
            , i18n("No fileName sent from calling method.");
            return;
} else if ( !QFileInfo(fileName).isReadable() ) {
    QMessageBox::critical(
            this
            , i18n("File not found")
            , i18n("%1 \nIs not readable", fileName.toString());
            return;
}
rizzitello: Im all for doing something like this where we show a different message for either error.
315 m_logWidget->appendLog(i18n("Printing:%1", fileName.toLocalFile())); 328​ m_logWidget->appendLog(i18n("Printing:%1", fileName.toLocalFile()));
316 m_core.print(fileName.toLocalFile()); 329​ m_core.print(fileName.toLocalFile());
317 } 330​ }
318} 331​}
319 332​
320void AtCoreInstanceWidget::print() 333​void AtCoreInstanceWidget::print()
321{ 334​{
322 emit(requestFileChooser()); 335​ emit(requestFileChooser());
▲ Show 20 Lines • Show All 285 Lines • Show Last 20 Lines