[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1735826 - in /subversion/trunk: ./ subversion/libsvn_subr/prompt.c

From: Julian Foad <julianfoad_at_apache.org>
Date: Thu, 31 Mar 2016 14:10:09 +0100

On 28 March 2016 at 14:49, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 28 March 2016 at 16:37, Stefan Sperling <stsp_at_apache.org> wrote:
>> On Mon, Mar 28, 2016 at 04:13:11PM +0300, Ivan Zhakov wrote:
>>> On 20 March 2016 at 01:18, <danielsh_at_apache.org> wrote:
>>> > {
>>> > if (err->apr_err == SVN_ERR_CANCELLED)
>>> > {
>>> > - svn_error_clear(err);
>>> > *may_save_plaintext = FALSE;
>>> > - return SVN_NO_ERROR;
>>> > + return err;
>>> Daniel, do you know what was the original idea behind ignoring the
>>> SVN_ERR_CANCELLED error? I see stsp committed the original code in
>>> r870804, so there's probably some rationale behind it.
>>>
>>> Stefan, do you remember any details?
>>
>> I don't think there was a special reason to ignore the error.
>>
>> The question is whether we want Ctrl-C to mean "no" at the plaintext
>> prompt, or whether it should abort the process.
>>
>> I agree with Daniel's patch. Ctrl-C should abort the process.
>
> OK. Thanks for clarification! I just wanted to double check that we're
> not missing something important.

In that case, why use the "err = prompt(...; if err == CANCELLED ..."
construction? If we're returning an error, then the assignment to
"*may_save_plaintext" should not be required, right? (Neither this
function nor its two public callers promise to do anything special
when returning an error.)

And so wouldn't it be equivalent and simpler to just write

    SVN_ERR(prompt(...));
?

- Julian
Received on 2016-03-31 15:10:32 CEST

This is an archived mail posted to the Subversion Dev mailing list.