[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Wed, 2011-06-01 at 16:18 -0400, Rob Crittenden wrote:
> 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

Thanks :-) Pushed to master.

Martin


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]