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

[Martin Kosek]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-069-3-improve-interactive-mode-for-dns-plugin.patch
Type: text/x-patch
Size: 18280 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110527/f595d107/attachment.bin>