On Fri, Feb 21, 2014 at 3:52 PM, Petr Pudlák <[email protected]> wrote:
> > > > On Tue, Feb 18, 2014 at 3:39 PM, Hrvoje Ribicic <[email protected]> wrote: > >> The PrintGenericInfo function in cli.py did not handle tuples as >> containers of items, making it impossible for these to be deserialized >> automatically when a YAML parser is used. This patch adds separate >> handling of tuples, including inlining them for readability when >> possible. >> >> Signed-off-by: Hrvoje Ribicic <[email protected]> >> --- >> lib/cli.py | 25 ++++++++++++++++++++++++- >> 1 file changed, 24 insertions(+), 1 deletion(-) >> >> diff --git a/lib/cli.py b/lib/cli.py >> index 951ef4a..97c16ba 100644 >> --- a/lib/cli.py >> +++ b/lib/cli.py >> @@ -4172,6 +4172,16 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None, >> return ipolicy_out >> >> >> +def _NotAContainer(data): >> + """ Checks whether the input is not a container data type. >> + >> + @rtype: bool >> + >> + """ >> + return not (isinstance(data, list) or isinstance(data, dict) or >> + isinstance(data, tuple)) >> > > At other places in the code I've seen > > isinstance(data, (list, dict, tuple)) > > perhaps this could be used as well (but I'm not so familiar with > isinstance to be 100% sure). > > Ack, I completely forgot about this syntax, thanks! > + >> + >> def _SerializeGenericInfo(buf, data, level, afterkey=False): >> """Formatting core of L{PrintGenericInfo}. >> >> @@ -4217,7 +4227,20 @@ def _SerializeGenericInfo(buf, data, level, >> afterkey=False): >> buf.write(key) >> buf.write(": ") >> _SerializeGenericInfo(buf, val, level + 1, afterkey=True) >> - elif isinstance(data, list): >> + elif isinstance(data, tuple) and all(map(_NotAContainer, data)): >> + # tuples with simple content are serialized as inline lists >> + first = True >> + buf.write('[') >> + for item in data: >> + if first: >> + first = False >> + else: >> + buf.write(", ") >> + buf.write(str(item)) >> + buf.write(']') >> + buf.write("\n") >> > > Perhaps this could be simplified something like this (untested): > > buf.write("[" + ", ".join(map(str, data)) + "]\n") > > or even better > > buf.write("[" + utils.CommaJoin(data) + "]\n") > > or (as I've seen at some place in our code) > > buf.write("[%s]\n" % utils.CommaJoin(data)) > > (at slight performance cost.) > Having written this exact code dozens of times, I do not know why I made this particular choice. Bad day, I guess :) Will fix! > > >> + elif isinstance(data, list) or isinstance(data, tuple): >> + # lists and tuples >> if not data: >> buf.write("\n") >> else: >> -- >> 1.9.0.rc1.175.g0b1dcb5 >> >> > Otherwise LGTM > Thanks for the review, interdiff is: diff --git a/lib/cli.py b/lib/cli.py index 97c16ba..a07417b 100644 --- a/lib/cli.py +++ b/lib/cli.py @@ -4178,8 +4178,7 @@ def _NotAContainer(data): @rtype: bool """ - return not (isinstance(data, list) or isinstance(data, dict) or - isinstance(data, tuple)) + return not (isinstance(data, (list, dict, tuple))) def _SerializeGenericInfo(buf, data, level, afterkey=False): @@ -4229,16 +4228,7 @@ def _SerializeGenericInfo(buf, data, level, afterkey=False): _SerializeGenericInfo(buf, val, level + 1, afterkey=True) elif isinstance(data, tuple) and all(map(_NotAContainer, data)): # tuples with simple content are serialized as inline lists - first = True - buf.write('[') - for item in data: - if first: - first = False - else: - buf.write(", ") - buf.write(str(item)) - buf.write(']') - buf.write("\n") + buf.write("[%s]\n" % utils.CommaJoin(data)) elif isinstance(data, list) or isinstance(data, tuple): # lists and tuples if not data:
