Jan Zelený wrote:
Ok, I'm sending updated patch in attachment

Should I change it in class help then? That's where I copied this from.

I think so.

Ok, I'll send another patch, so me don't mix it together with this patch. I'll
do a review of the code in cli.py, maybe the same issue is elsewhere as well.

This will blow up as expected in the FIXME if an unknown command is
passed in.

Fixed, thanks.

Not to be pedantic but I think it should return a non-zero error code
too on error.

Yep, replaced this with exception.

ipa show-mappings user-show returns just 'rights'

If it was the acting correctly, it shouldn't be displayed at all, because
it is not LDAP based (and user-show doesn't take any other LDAP-based
arguments/options).

I'm just not sure how to do this with minimal changes. One option is to
create new flag denoting whether parameter is LDAP based or not and for
each parameter set it appropriately, but that is just too much effort
for something that is not that important. That's why I use the 'webui'
flag to filter things at least a little bit.

You should have the object Params list available, right? Can you use
that to show at least some attributes?

I already thought of that, but that would add only primary key, since Params
is a concatenation of Options and Args - in args there are usually only
mandatory arguments (i.e. primary keys, uid in case of user-show) and options
are already iterated over and printed out.

I think adding this is too much effort. For one thing user-show takes no other
options than --rights (and the purpose of the patch is to show mapping between
CLI options and LDAP attributes) and user can always see real LDAP attributes
of user object by using --raw.

Jan

Just a really minor nit. Can you define a label for the argument? Otherwise if you run: `ipa show-mappings` it will prompt for <command_name>.

rob

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to