pinoaffe / org-vcard

Export and import vCards from within GNU Emacs' Org mode.
63 stars 7 forks source link

Support QUOTED-PRINTABLE fields #30

Closed wi11dey closed 5 years ago

wi11dey commented 5 years ago

Contact exports on Android use and ENCODING=QUOTED-PRINTABLE encoding for folded lines, text with line breaks, and non-ASCII characters. These end up all over my export files.

Thankfully Emacs has built-in with support for encoded/decoding quoted-printable in the qp feature. This PR adds support for decoding such fields properly, loading the qp feature only when necessary.

org-vcard-import-parse was reworked in the process while keeping the same functionality. Some duplicate regexes have been consolidated and the append that was bringing the overall time to O(n2) has been switched to O(n) push + nreverse. Feel free to change the comment indentation.

All tests pass, and another has been added for quoted-printable

wi11dey commented 5 years ago

Looks like this will also close #25 since technician77's contact was failing because of the line breaks quoted-printable can introduce (see https://github.com/flexibeast/org-vcard/issues/25#issuecomment-331609788)

flexibeast commented 5 years ago

Sorry to have not responded sooner.

This looks great - thanks for your contribution!

A couple of small, entirely subjective, nitpicks, regarding the let for org-vcard-import-parse:

Putting all that together, i'd prefer the let to be:

(let ((current-line "")
      (property "")
      (value "")
      (charset "")
      (encoding "")
      (cards '())
      (current-card '()))

Other than that, if you could please add yourself to the copyright and author lines, and change the end of the copyright range from '2017' to '2019', i'll then merge this. :-)

Thanks again!

wi11dey commented 5 years ago

Thanks for your reply.

This implementation doesn't use the current-line variable but I forgot to remove a setq current-line that wasn't doing anything. There's no references to it anymore so I've removed it from the varlist.

The let looks like this now:

(let ((property "")
      (value "")
      (charset "")
      (encoding "")
      (cards '())
      (current-card '()))

That's a neat convention - I will consider it in future projects.

PR should be up to standard now. When I get some time I'll be making some more requests :)

flexibeast commented 5 years ago

I forgot to remove a setq current-line that wasn't doing anything. There's no references to it anymore so I've removed it from the varlist.

:-)

PR should be up to standard now. When I get some time I'll be making some more requests :)

Yep, all looks good now, so i've merged. Thanks again! Look forward to your future PRs. :-)