[Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin

[Rob Crittenden]

Martin Kosek wrote:
> On Fri, 2011-05-27 at 16:25 -0400, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote:
>>>> Martin Kosek wrote:
>>>>> Interactive mode for commands manipulating with DNS records
>>>>> (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances
>>>>> the server framework with new callback for interactive mode, which
>>>>> can be used by commands to inject their own interactive handling.
>>>>>
>>>>> The callback is then used to improve aforementioned commands'
>>>>> interactive mode.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/1018
>>>>
>>>> This works pretty nicely but it seems like with just a bit more it can
>>>> be great.
>>>>
>>>> Can you add some doc examples for how this works?
>>>
>>> Done. At least user will know that we have a feature like that to offer.
>>>
>>>>
>>>> And you display the records now and then prompt for each to delete. Can
>>>> you combine the two?
>>>>
>>>> For example:
>>>>
>>>> ipa dnsrecord-del greyoak.com lion
>>>> No option to delete specific record provided.
>>>> Delete all? Yes/No (default No):
>>>> Current DNS record contents:
>>>>
>>>> A record: 192.168.166.32
>>>>
>>>> Enter value(s) to remove:
>>>> [A record]:
>>>>
>>>> If we know there is an record why not just prompt for each value yes/no
>>>> to delete?
>>>
>>> Actually, this is a very good idea, I like it. I updated the patch so
>>> that the user can only do yes/no decision in ipa dnsrecord-del
>>> interactive mode. This makes dnsrecord-del interactive mode very usable.
>>>
>>>>
>>>> The yes/no function needs more documentation on what default does too.
>>>> It appears that the possible values are None/True/False and that None
>>>> means that '' can be returned (which could still be evaluated as False
>>>> if this isn't used right).
>>>
>>> Done. '' shouldn't be returned as I return the value of "default" if it
>>> is not None. But yes, it needed more documenting.
>>>
>>> Updated patch is attached. It may need some language corrections, I am
>>> no native speaker.
>>>
>>> Martin
>>
>> Not to be too pedantic but...
>>
>> The result variable isn't really used, a while True: would suffice.
>>
>> I'm not really sure what the purpose of default = None is. I think a
>> True/False is more appropriate, this 3rd answer of a binary question is
>> confusing.
>
> I fixed the result variable. This was a left-over from function
> evolution.
>
> I am not sure why is the yes/no function still confusing. Maybe I miss
> something. I improved function help a bit. But let me explain:
>
> If default is None, that means that there is no default answer to yes/no
> question and user has to answer either "y" or "n". He cannot skip the
> answer and is prompted until the answer is given.
>
> When default is True, user can just enter empty answer, which is treated
> as "yes" and True is returned.
>
> When default is False and user enters empty answer, it is treated as
> "no" and False is returned.
>
> None shouldn't be returned at all... (Maybe only in a case of an error)
>
> Martin
>

Wow, this is very nice indeed. Ack.

rob