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

[Rob Crittenden]

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.

rob