Check if file exists before print
ClosedPublic

Authored by laysrodrigues on Aug 28 2018, 12:37 PM.

Diff Detail

Repository
R231 Atelier
Branch
check_if_path_exists
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 2581
Build 2599: arc lint + arc unit
laysrodrigues requested review of this revision.Aug 28 2018, 12:37 PM
laysrodrigues created this revision.
rizzitello requested changes to this revision.Aug 28 2018, 12:50 PM
rizzitello added inline comments.
src/widgets/atcoreinstancewidget.cpp
314–329

Its a shame QFileInfo does not accept a QUrl :(

315

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.

This revision now requires changes to proceed.Aug 28 2018, 12:50 PM

Making sith happy
+ rebase

rizzitello accepted this revision.Sep 5 2018, 1:24 AM
This revision is now accepted and ready to land.Sep 5 2018, 1:24 AM
patrickelectric requested changes to this revision.Sep 5 2018, 1:51 AM
patrickelectric added inline comments.
src/widgets/atcoreinstancewidget.cpp
314–329

also !QFileInfo(fileName)::isReadable()

This revision now requires changes to proceed.Sep 5 2018, 1:51 AM
laysrodrigues added inline comments.Sep 5 2018, 2:29 AM
src/widgets/atcoreinstancewidget.cpp
314–329

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()) {
rizzitello requested changes to this revision.Sep 5 2018, 11:34 AM
rizzitello added inline comments.
src/widgets/atcoreinstancewidget.cpp
314–329

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 added inline comments.Sep 6 2018, 12:19 AM
src/widgets/atcoreinstancewidget.cpp
314–329

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;
}

patrick and sith suggestions

rizzitello accepted this revision.Sep 7 2018, 8:06 PM

change endline of MessageBox

rizzitello accepted this revision.Sep 7 2018, 8:22 PM
patrickelectric accepted this revision.Sep 7 2018, 9:01 PM
This revision is now accepted and ready to land.Sep 7 2018, 9:01 PM
laysrodrigues closed this revision.Sep 8 2018, 1:29 PM