Re: [PATCH master 16/26] Make PrintGenericInfo handle tuples better

[Petr Pudlák]

On Tue, Feb 18, 2014 at 3:39 PM, Hrvoje Ribicic <r...@google.com> 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 <r...@google.com>
> ---
>  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).


> +
> +
>  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.)


> +  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