Add option --raw for the show command.
ClosedPublic

Authored by dfaure on Jul 9 2019, 7:53 PM.

Details

Summary

Encoded (raw) email payloads are not that useful.
In my use case, I'm extracting contents from emails in a perl script,
so doing the decoding using KMime is quite convenient.

Test Plan

$ akonadiclient show --raw 1612234 |& grep '^Postage'
Postage & Packing: =C2=A34.23

$ akonadiclient show 1612234 |& grep '^Postage'
Postage & Packing: £4.23

Diff Detail

Repository
R723 Akonadi CLI Client
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14521
Build 14539: arc lint + arc unit
dfaure requested review of this revision.Jul 9 2019, 7:53 PM
dfaure created this revision.
marten added inline comments.Jul 9 2019, 9:22 PM
src/showcommand.cpp
37

Should have a short option as well (all other subcommands/options do). Maybe '-d'?

dfaure updated this revision to Diff 61471.Jul 9 2019, 10:30 PM

Add -d short option (tested)

krake added a comment.Jul 10 2019, 6:17 AM

I am very much in favor of adding this but I am thinking we probably want this a bit more generic.
For a console tool the commandline switches are very much like API, so having an email specific switch doesn't "feel" right.

For kabcclient, which is kind of the precursor of this, we supported various input/output formats and format specific options through generic commandline switches:
http://manpages.ubuntu.com/manpages/trusty/en/man1/kabcclient.1.html
https://phabricator.kde.org/source/kdepim/browse/master/console/kabcclient/src/formatfactory.cpp;ee3dfc152d1d71df22b60bdcdee317fb9997ab97

We also had a tool for calendar called konsolekalendar which also had different types of output
https://phabricator.kde.org/source/kdepim/browse/master/console/konsolekalendar/konsolekalendarexports.cpp;ee3dfc152d1d71df22b60bdcdee317fb9997ab97

"raw" would work across all payload types, maybe even something like "human".

Main difficulty I see is that the "show" command supports a list of items and they don't necessarily need to have the same payload type.
So I am a bit torn between "output-format" and something like "email-format", "contact-format" and so on

Good point regarding the possibility of decoding other formats.

Perhaps the option could be renamed simply "--decoded", with the action being that it attempts to decode the payload as far as possible into a human readable format. Currently the only format that it would actually know how to decode would be emails, but there is always the possibility of decoding other formats later with no change needed to the command line options or syntax.

"Quoted Printable" is essentially just about output encoding, email just has this as the default while VCard V3, for example, has UTF-8.

If "local8Bit" is different to UTF-8 then even a VCard/Contact would need to be "decoded".
So in kabcclient that was even another option, "output codec".

We are currently lucky most people have UTF-8 as the local encoding.

Maybe we should take this the other way around:

  • make a switch --raw: no processing of payload data
  • if not present, "decode" into QString, then use toLocal8Bit() or qPrintable() to correctly encode for the program's environment

We can then later on add "output codec" options if necessary.

Would also make it easier for David right now, essentially just inverting the logic of his flag. Something like

if (mRawPayload) {

std::cout << item.payloadData().constData();

} else {

if (email) {
} else {
    std::cout << qPrintable(QString::fromUtf8(item.payloadData().constData));
}

}

dfaure added a comment.EditedJul 11 2019, 7:40 AM

I don't think output codec is needed, toLocal8Bit/qPrintable is the expected output codec for commandline programs.

So the current two ideas are --decoded (default raw) and --raw (default decoded). I chose the former to avoid breaking existing users, but if you guys prefer, I'm also OK with the latter.
This is playground after all (unfortunately...).

And yes if someone wants to choose between vcard and csv for contacts, that would be a --contact-format switch. This way it works even in an unlikely mixed folder, but more importantly it's easier to document (contact-format lists the contact formats, event-format lists the event formats etc.).

Hmm, does this mean instead of --decoded, what I want is --email-format decoded ?

krake added a comment.Jul 11 2019, 8:01 AM

I don't think output codec is needed, toLocal8Bit/qPrintable is the expected output codec for commandline programs.

Output codec was mostly important for certain outputs, e.g. VCard 2, which doesn't have a standard codec.
For now I agree we should be fine with either of these two methods of creating "local" output.

So the current two ideas are --decoded (default raw) and --raw (default decoded). I chose the former to avoid breaking existing users, but if you guys prefer, I'm also OK with the latter.

I prefer "raw" as the exception due to the command being "show", i.e. "human consumable".
"raw" is more like "internal format, you have been warned".

This is playground after all (unfortunately...).

Originally I was planning to get it into pim/console just like kabcclient and konsolecalendar used to, but the multi data nature makes it much more complex than these two and I never got around to really think about the API in enough detail to risk "official" releasing.

And yes if someone wants to choose between vcard and csv for contacts, that would be a --contact-format switch. This way it works even in an unlikely mixed folder, but more importantly it's easier to document (contact-format lists the contact formats, event-format lists the event formats etc.).

Documentation/discoverability is a good point.

For the generic switch I was thinking maybe something like

--output-format email=full,contact=vcard

i.e. allowing the argument to be a list of sorts.

Encountering different types of items is unlikely, but the command allows to specify a list of item IDs, which could even come from different collections.
So it is a decision of either not allowing/supporting different types in the same invocation or being capable of specifying options individually.

Hmm, does this mean instead of --decoded, what I want is --email-format decoded ?

That was my initial take, but now I think quoted printables should always be decoded unless specifically requested not to.
I.e. if you want to process the item's payload with a mail capable program you simply use "--raw", if you are manually parsing information of any sort you probably don't want to deal with quoted printable encoding yourself.

dfaure updated this revision to Diff 62658.Jul 27 2019, 6:43 PM
dfaure retitled this revision from Add option --decode-emails for the show command. to Add option --raw for the show command..
dfaure edited the summary of this revision. (Show Details)
dfaure edited the test plan for this revision. (Show Details)

Name the (reverse) option --raw

krake requested changes to this revision.Jul 27 2019, 10:31 PM

Thanks!

One minor wish added :-)

src/showcommand.cpp
105

Can I ask to move the mRaw check to else above?
I.e. make it

} else if (mRaw) {
} else {
}

For non-emails the "not raw" should then do something like

std::cout << qPrintable(QString::fromUtf8(item.payloadData().constData());

Rational: if the local encoding is not UTF-8 then "raw" would still write the internal UTF-8 data (raw == not modified or recoded) but the "not raw" branch would recode to local8Bit

This revision now requires changes to proceed.Jul 27 2019, 10:31 PM
dfaure updated this revision to Diff 62713.Jul 29 2019, 7:49 AM

Add non-raw output for non-emails.

AFAIK all Linux distros use utf-8 so this really only makes a difference on Windows...

krake accepted this revision.Jul 29 2019, 8:35 AM

Perfect!
Thanks a lot!

This revision is now accepted and ready to land.Jul 29 2019, 8:35 AM
dfaure closed this revision.Jul 29 2019, 8:58 AM