Sorry about my delay in answering...

Chong Yidong <cyd@stupidchicken.com> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>>     I made "status" an extra argument at the beginning of the argument
>>     list (so if CBARGS has N elements, the callback is called with N+1
>>     arguments).  I described this in the docstring of url-retrieve in my
>>     patch below (not yet committed).
>>
>> Is this new argument _unconditionally_ present?
>
>>From my reading of the code, it is unconditionally present (though it
> can be an empty list).

Exactly.

>> I can't tell, and I see comments that seem to suggest
>> that it might not be so:
>>
>> +       (setf (car url-callback-arguments)
>> + 	    (nconc (list :error (list 'error 'connection-failed why
>> + 				      :host (url-host url-current-object)
>> + 				      :service (url-port url-current-object)))
>> + 		   (car url-callback-arguments)))
>
> The car of url-callback-arguments is the new STATUS argument, so this
> is correct.

Yes.

>> And this:
>>
>>       It is called as (apply CALLBACK STATUS CBARGS), where STATUS
>>     ! is a list with an even number of elements representing what happened
>>     ! during the request, with most recent events first.  Each pair is one
>>     ! of:
>>     ! 
>>     ! \(:redirect REDIRECTED-TO) - the request was redirected to this URL
>>     ! \(:error (ERROR-SYMBOL . DATA)) - an error occurred.  The error can be
>>     ! signaled with (signal ERROR-SYMBOL DATA).
>
> This is pretty clear: it says there is a STATUS argument; it
> definitely doesn't say that argument can be elided (and it isn't).

Yes, that's what I wanted to say.

>> I don't like that variability, and it is easy to avoid.  If we are to
>> change the API of these callbacks now, let's change it to something
>> more consistent: add a single argument unconditionally.  That added
>> argument can be a property list in which :redirect and :error may
>> occur.
>
> I don't see anything wrong with the patch.  I suggest checking it in
> and beginning the pretest.  OK?

I found that the patch breaks url-retrieve-synchronously, because of
the removal of url-redirect-buffer.  It would be cleaner to remove it
and fix url-retrieve-synchronously to use something else, but now is
not the time, I think.  The attached patch replaces my previous one.
Should I commit it?