KSieve: redesign to fix error handling.
ClosedPublic

Authored by dfaure on Jul 15 2016, 10:06 PM.

Details

Summary
  • Fix parsing of "NO {123}" (followed by error lines). The thread separation

broke Volker's "feedback" hack^H^H^H^Hdesign, because the state is wrong at the
time the reparsing happens. This syntax is now handled during response parsing
rather than being fed back to the thread.

  • Fix double messagebox on error, because on destruction the job would call

killJob which would emit the error again. To fix this I followed more closely
the KJob API (even though SieveJob isn't a KJob), using the (currently unused)
KillVerbosity argument.

  • Fix some error messages being stored in Session and never shown, now those

are given to the current job. This also goes closer to KJob: errorString()
in the job rather than an errorMessage signal.

  • Finally, remove messageboxes shown by the job itself (and setInteractive(false)

to disable that), let the caller show a messagebox on error if it wants to
(again, more like KJob). This fixes ugly reentrancy problems while the msgbox
is up from within the job code, and allows to customize the error messages better.

Test Plan

Editing a sieve script (on mykolab.org, which uses the NO {123} syntax)
and introducing an error, then using "check syntax" (->error at bottom),
and OK (->error message box). This also makes "check syntax" show green again
after fixing the script (previously the first job prevented any other job from running).

Diff Detail

Repository
R91 PIM: Sieve Handling Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dfaure updated this revision to Diff 5220.Jul 15 2016, 10:06 PM
dfaure retitled this revision from to KSieve: redesign to fix error handling..
dfaure updated this object.
dfaure edited the test plan for this revision. (Show Details)
dfaure added reviewers: dvratil, vkrause, mlaurent.
dfaure added a subscriber: KDE PIM.
Restricted Application added a project: KDE PIM. · View Herald TranscriptJul 15 2016, 10:06 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
mlaurent edited edge metadata.Jul 16 2016, 7:49 AM

I tried it on sieveeditor but we can"t as API changed.

> you need to adapt sieveeditor + increase libksieve version + all set(LIBKSIEVE_LIB_VERSION_LIB "5.2.82") in kdepim (not just on toplevel :) )

Regards

Otherwise it seems to work correctly in kmail.

dfaure updated this revision to Diff 5224.Jul 16 2016, 9:00 AM
dfaure edited edge metadata.

Increase libksieve version

mlaurent accepted this revision.Jul 16 2016, 10:37 AM
mlaurent edited edge metadata.

Seems ok for me :)
Thanks

This revision is now accepted and ready to land.Jul 16 2016, 10:37 AM
dvratil accepted this revision.Jul 19 2016, 7:06 PM
dvratil edited edge metadata.
This revision was automatically updated to reflect the committed changes.