On 4/17/14, 10:42 PM, Steve Holme wrote:
> On Thu, 17 Apr 2014, Dionysios Kalofonos wrote:
>
> Now I appreciate that we don't have it correct at the moment by
> imitating a LIST command, and I appreciate you are trying to fix my
> decision there ;-) What you have implemented is certainly a lot
> better than that but if we view a custom request as the ability to
> override any command then we should also allow APPEND (the upload) to
> be overridden by a custom request as well.

I agree, APPEND should also be handled by a custom request. If we decide
to continue with the custom state approach i will fix it.

>
> I wouldn't like to proceed down the "custom" state / perform / resp
> function route only to rewrite it for a third time in the future
> without airing some other possibilities ;-)
>
> As such.... Is there a way that we can specify the custom request but
> call the existing functions?
>
> For example:
>
> * Modify the decision process in imap_perform() to ignore the custom
> variable. * In imap_perform_fetch(), imap_perform_list(),
> imap_perform_append() we send the contents of custom if the custom
> variable is set and send the existing command if it isn’t? * I
> appreciate the responses may be a little different and that might
> make the imap_endofresp() function a little complex :(

I agree, using the existing response handlers would have been the best
approach. However, when i saw that the FETCH response handler parses the
responses i decided that it would have been better having a custom
state. I will discuss my concerns with parsing the responses, by
presenting some alternatives that i can think of, with their pros/cons.

A) Modify the response handlers so that the only parsing/processing they
do to the responses is to handle string literals i.e. the {<count>}\r\n case

http://tools.ietf.org/html/rfc3501#section-4.3

Hence the client receives complete responses. But not what the current
fetch implementation does, which extracts and returns only the string
literal.

Currently,

< * 16 FETCH (FLAGS (\Seen) BODY[1] {334}
* Found 334 bytes to download
U3RhcnRzOldlZG5lc2Rhe...
* Written 334 bytes, 0 bytes are left for transfer
< )

Desirable,

* 16 FETCH (FLAGS (\Seen) BODY[1] {334}
U3RhcnRzOldlZG5lc2Rhe...
)

The benefits are 1) the API will be minimal, 2) the client will receive
strings complying with the IMAP spec, and 3) the role of CURL is clear.

The negatives are the API/ABI changes.

B) Modify the response handlers so that on custom requests they return
the responses verbatim to the client. Otherwise, they parse the
responses as they do now.

This is basically logically equivalent to having a custom state with its
actuators/response handlers as the patch i've sent you.

The benefits are 1) we maintain backward compatibility, 2) we maintain a
small API that is able to handle every form of response.

The negatives are that a custom FETCH 1 BODY[1] will return to the
client different results from the same request made through the URL
imap://imap.example.com/INBOX/;UID=1/;SECTION=1.

I agree with you, this is horribly ugly.

C) We extend the API so that the response handlers can parse any
response from the server, following the theme of the existing
implementation. I think this is where it gets hairy.

The immediate question is how much parsing/processing should we do to
the returned responses? Where should the line be drawn between returning
the responses verbatim to building a feature rich email client?

But lets say for the sake of argument, that we indeed build a feature
rich email client, and that we are able to parse every kind of response,
encodings, etc.

The only structure between CURL and the client is a string/file through
the CURLOPT_WRITEFUNCTION and CURLOPT_WRITEDATA. The question is, having
parsed the responses and extracted/decoded the tokens, how should we
structure the string returned to the client so that it is parseable by
the client?

Moreover, when we return the responses verbatim to the client, the
client has the IMAP spec as a reference. If we return hand made strings
then these will become part of the API.

Sorry for not having any answers. I think A is the best approach, even
though it requires API changes.

>
> B) Coding Style
>
> Generally speaking your coding style doesn't match that of curls:
>
> * Curly brackets for code segments such as if statements are not in
> the correct place for curl. We follow the old school way of doing it
> for those blocks and have the start brace on the end of the
> if/for/while statement. * Whilst we have a mixture of code that
> quickly exits a function after a test condition and code that
> performs if/else I have tried to maintain a consistent style in the
> email protocols. I would recommend the if in imap_endofresp() be an
> if/else and let the return TRUE at the end of the function execute in
> it's natural code path. * We also have a mix of placing the constant
> of an if condition on the left and on the right - again I have tried
> to be consistent in the email protocols using RHS rather than LHS. *
> You have used a mixture of tabs and spaces for indentation - please
> use spaces. * The indentation of imap_state_custom_resp() seems a
> little strange - quite possible if you have tabs set as 8 characters
> in your editor?
>
> Other than that it's a pretty good first stab ;-)

Sorry, i should have paid more attention. I will fix those once we
decide to continue with the custom state approach.

Thank you for your time and feedback.

Kind regards,

-- 
Dionysios Kalofonos
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html