Re: [Freeipa-devel] [PATCH] Fix output of commands, that do not return entries.
Pavel Zůna wrote: Rob Crittenden wrote: Pavel Zůna wrote: Pavel Zůna wrote: This patch fixes Command.output_for_cli for the env plugin. Before we assumed, that a list/tuple is always a list of entries and a dict is always an entry. Still, this solution isn't perfect. I think, that in the future, we should allow Output subclasses to control the way we output values instead of doing type-based output in Command.output_for_cli. Pavel Before anyone asks... :) I also changed the default value of the print_all argument in textui.print_entry from False to True. It think it makes more sense this way, because: 1) if order is None, it will still print something 2) if order is not None, it will print what's in order first and then the rest 3) commands that care about the print_all argument have to set it in any case, those that don't care usually want to print everything Why not set the default for print_all in print_entries() to True as well? That's just a mistake I made. Fixed. Seems like this reasoning should be documented in the function as well. Particularly how print_all gets handled when one returns Entries or a ListOfEntries vs just returning a dict/tuple (where --all controls whether everything is printed in the former and defaults to everything in the later assuming print_entries also ends up defaulting to True). Added docstring for Command.output_for_cli and also updated the docstring for Command.get_options with info about --all/--raw. rob New patch attached. Pavel Ok. I'm a little uneasy about printing everything but we'll address that case when it comes up. So far everything works. I added your reasoning to the commit message. ack, pushed to master. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix output of commands, that do not return entries.
Rob Crittenden wrote: Pavel Zůna wrote: Pavel Zůna wrote: This patch fixes Command.output_for_cli for the env plugin. Before we assumed, that a list/tuple is always a list of entries and a dict is always an entry. Still, this solution isn't perfect. I think, that in the future, we should allow Output subclasses to control the way we output values instead of doing type-based output in Command.output_for_cli. Pavel Before anyone asks... :) I also changed the default value of the print_all argument in textui.print_entry from False to True. It think it makes more sense this way, because: 1) if order is None, it will still print something 2) if order is not None, it will print what's in order first and then the rest 3) commands that care about the print_all argument have to set it in any case, those that don't care usually want to print everything Why not set the default for print_all in print_entries() to True as well? That's just a mistake I made. Fixed. Seems like this reasoning should be documented in the function as well. Particularly how print_all gets handled when one returns Entries or a ListOfEntries vs just returning a dict/tuple (where --all controls whether everything is printed in the former and defaults to everything in the later assuming print_entries also ends up defaulting to True). Added docstring for Command.output_for_cli and also updated the docstring for Command.get_options with info about --all/--raw. rob New patch attached. Pavel 0001-Fix-output-for-commands-that-do-not-return-entries.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix output of commands, that do not return entries.
Pavel Zůna wrote: Pavel Zůna wrote: This patch fixes Command.output_for_cli for the env plugin. Before we assumed, that a list/tuple is always a list of entries and a dict is always an entry. Still, this solution isn't perfect. I think, that in the future, we should allow Output subclasses to control the way we output values instead of doing type-based output in Command.output_for_cli. Pavel Before anyone asks... :) I also changed the default value of the print_all argument in textui.print_entry from False to True. It think it makes more sense this way, because: 1) if order is None, it will still print something 2) if order is not None, it will print what's in order first and then the rest 3) commands that care about the print_all argument have to set it in any case, those that don't care usually want to print everything Why not set the default for print_all in print_entries() to True as well? Seems like this reasoning should be documented in the function as well. Particularly how print_all gets handled when one returns Entries or a ListOfEntries vs just returning a dict/tuple (where --all controls whether everything is printed in the former and defaults to everything in the later assuming print_entries also ends up defaulting to True). rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix output of commands, that do not return entries.
Pavel Zůna wrote: This patch fixes Command.output_for_cli for the env plugin. Before we assumed, that a list/tuple is always a list of entries and a dict is always an entry. Still, this solution isn't perfect. I think, that in the future, we should allow Output subclasses to control the way we output values instead of doing type-based output in Command.output_for_cli. Pavel Before anyone asks... :) I also changed the default value of the print_all argument in textui.print_entry from False to True. It think it makes more sense this way, because: 1) if order is None, it will still print something 2) if order is not None, it will print what's in order first and then the rest 3) commands that care about the print_all argument have to set it in any case, those that don't care usually want to print everything Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
