Use struct to parse/read vacation script.
ClosedPublic

Authored by knauss on Oct 23 2015, 4:54 PM.

Diff Detail

Repository
R43 KDE PIM
Lint
No Linters Available
Unit
No Unit Test Coverage
knauss updated this revision to Diff 1040.Oct 23 2015, 4:54 PM
knauss retitled this revision from to Use struct to parse/read vacation script..
knauss added reviewers: mlaurent, vkrause.
knauss added a project: KDE PIM.
vkrause added inline comments.Oct 24 2015, 9:14 AM
libksieve/src/ksieveui/vacation/vacationutils.cpp
205

This seems unused?

vkrause added inline comments.Oct 24 2015, 9:42 AM
libksieve/src/ksieveui/vacation/vacationutils.cpp
205

nm, addressed by D448.

mlaurent added inline comments.Oct 25 2015, 1:27 PM
libksieve/src/ksieveui/vacation/autotests/vacationutilstest.cpp
35

const int l1count = l1.count();

> avoid to recall it.

libksieve/src/ksieveui/vacation/vacationutils.cpp
163–166

you "parse" twice ?

381

const

392

const

479

const QString &scriptupdate

507

++i

517

const int requirementcount = requirements.count(),

knauss updated this revision to Diff 1054.Oct 26 2015, 10:50 AM
knauss marked 9 inline comments as done.

update with laurents comments & running astyle.

knauss updated this revision to Diff 1057.Oct 26 2015, 1:15 PM

move patch part to this patch.

mlaurent added inline comments.Oct 26 2015, 8:51 PM
libksieve/src/ksieveui/vacation/autotests/vacationutilstest.cpp
306

++i

311

++i

libksieve/src/ksieveui/vacation/vacationutils.cpp
289

Better to write all in one line and not 3.
QString script = (...).arg(...)

294

isEmpty()

320

move this line after the if (scriptUtf8.isEmpty()) not necessary to generate scriptUpdateUtf8 if we will not use it.

398

++i

knauss updated this revision to Diff 1063.Oct 27 2015, 11:46 AM
knauss marked 6 inline comments as done.

update with laurents comments

mlaurent added inline comments.Oct 30 2015, 6:01 AM
libksieve/src/ksieveui/vacation/autotests/CMakeLists.txt
18

Are you sure that "\\" is still necessary in kf5 ?

libksieve/src/ksieveui/vacation/vacationutils.cpp
185

why QString::fromAscii(...).toUtf8() ? why not QByteArray directly ? QByteArrayLiteral ?
Same for 'Vacation' you create a QStringLiteral and use toUtf8() after thar

253

remove space between begin() and ";"

knauss updated this revision to Diff 1134.Nov 2 2015, 11:44 AM
knauss marked 3 inline comments as done.

update

knauss added inline comments.Nov 2 2015, 12:05 PM
libksieve/src/ksieveui/vacation/autotests/CMakeLists.txt
18

well if using set_target_properties: yes, but switching to add_defintion there is no need to use it anymore.

mlaurent edited edge metadata.Nov 4 2015, 5:45 AM

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationcheckjob.cpp: In member function ‘void KSieveUi::VacationCheckJob::slotGetResult(KManageSieve::SieveJob*, bool, const QString&, bool)’:
/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationcheckjob.cpp:55:9: warning: unused variable ‘dummyInt’ [-Wunused-variable]

int dummyInt;
    ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationcheckjob.cpp:56:10: warning: unused variable ‘dummyBool’ [-Wunused-variable]

bool dummyBool;
     ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp: In function ‘KSieveUi::VacationUtils::Vacation KSieveUi::VacationUtils::parseScript(const QString&)’:
/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:163:33: error: ‘class KSieveUi::VacationDataExtractor’ has no member named ‘commandFound’

if (!parser.parse() || !vdx.commandFound()) {
                            ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:169:27: error: ‘class KSieveUi::VacationDataExtractor’ has no member named ‘active’

vacation.active = vdx.active();
                      ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:170:31: error: ‘class KSieveUi::VacationDataExtractor’ has no member named ‘mailAction’

vacation.mailAction = vdx.mailAction();
                          ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:171:40: error: ‘class KSieveUi::VacationDataExtractor’ has no member named ‘mailActionRecipient’

vacation.mailActionRecipient = vdx.mailActionRecipient();
                                   ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:184:34: error: ‘class KSieveUi::VacationDataExtractor’ has no member named ‘ifComment’

if (!vacation.active && !vdx.ifComment().isEmpty()) {
                             ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:185:69: error: ‘class KSieveUi::VacationDataExtractor’ has no member named ‘ifComment’

const QByteArray newScript = QByteArrayLiteral("if ") + vdx.ifComment().toUtf8() + QByteArrayLiteral("{vacation;}");
                                                            ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:199:29: error: ‘class KSieveUi::DateExtractor’ has no member named ‘startTime’

vacation.startTime = dx.startTime();
                        ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:201:27: error: ‘class KSieveUi::DateExtractor’ has no member named ‘endTime’

vacation.endTime = dx.endTime();
                      ^

/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp: In function ‘QString KSieveUi::VacationUtils::mergeRequireLine(const QString&, const QString&)’:
/stockage/kde5/kdepim/libksieve/src/ksieveui/vacation/vacationutils.cpp:330:5: error: ‘RequireExtractor’ was not declared in this scope

impossible to test compile for it.
I will add more patch now.

knauss updated this revision to Diff 1168.Nov 4 2015, 5:44 PM
knauss edited edge metadata.

removed unnessary viriables.

Plese keep in mind to compile this patch after D445.

mlaurent accepted this revision.Nov 6 2015, 6:02 AM
mlaurent edited edge metadata.

Seems ok now

This revision is now accepted and ready to land.Nov 6 2015, 6:02 AM
This revision was automatically updated to reflect the committed changes.