[PATCH] python: Provide textual representation for Inferior and Objfile

[PATCH] python: Provide textual representation for Inferior and Objfile

[Simon Marchi]

Printing a GDB Python object is notoriously not helpful:

>>> print(gdb.selected_inferior())
<gdb.Inferior object at 0x7fea59aed198>
>>> print(gdb.objfiles())
[<gdb.Objfile object at 0x7fea59b57c90>]

This makes printing debug traces more difficult than it should be.  This
patch provides some repr() implementation for these two types (more to
come if people agree with the idea, but I want to test the water first).
Here's the same example as above, but with this patch:

>>> print(gdb.selected_inferior())
<gdb.Inferior num=1>
>>> print(gdb.objfiles())
[<gdb.Objfile filename=/home/emaisin/build/binutils-gdb-gcc-git/gdb/test>]

I implemented repr rather than str, because when printing a list (or
another container I suppose), Python calls the repr method of the
elements.  This is useful when printing a list of inferiors or objfiles.
The print(gdb.objfiles()) above would not have worked if I had
implemented str.

I found this post useful to understand the difference between repr and
str:

  https://stackoverflow.com/questions/1436703/difference-between-str-and-repr

In your opinion, does this require an announcement in NEWS and an entry
in the manual?  I don't think the output of repr() should be considered
stable, as we could change it over time to make it more useful.  Maybe
that should be documented?

gdb/ChangeLog:

	* python/py-inferior.c (infpy_repr): New.
	(inferior_object_type): Register infpy_repr.
	* python/py-objfile.c (objfpy_repr): New.
	(objfile_object_type): Register objfpy_repr.

gdb/testsuite/ChangeLog:

	* gdb.python/py-inferior.exp: Test repr() of gdb.Inferior.
	* gdb.python/py-objfile.exp: Test repr() of gdb.Objfile.
	* gdb.python/py-symtab.exp: Update test printing an objfile.
---
 gdb/python/py-inferior.c                 | 17 ++++++++++++++++-
 gdb/python/py-objfile.c                  | 17 ++++++++++++++++-
 gdb/testsuite/gdb.python/py-inferior.exp | 14 +++++++++++++-
 gdb/testsuite/gdb.python/py-objfile.exp  |  7 +++++++
 gdb/testsuite/gdb.python/py-symtab.exp   |  2 +-
 5 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 1cf37296973..56019bf9e05 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -860,6 +860,21 @@ infpy_thread_from_thread_handle (PyObject *self, PyObject *args, PyObject *kw)
   return result;
 }
 
+/* Implement repr() for gdb.Inferior.  */
+
+static PyObject *
+infpy_repr (PyObject *obj)
+{
+  inferior_object *self = (inferior_object *) obj;
+  inferior *inf = self->inferior;
+
+  if (inf == nullptr)
+    return PyString_FromString ("<gdb.Inferior (invalid)>");
+
+  return PyString_FromFormat ("<gdb.Inferior num=%d, pid=%d>",
+			      inf->num, inf->pid);
+}
+
 
 static void
 infpy_dealloc (PyObject *obj)
@@ -991,7 +1006,7 @@ PyTypeObject inferior_object_type =
   0,				  /* tp_getattr */
   0,				  /* tp_setattr */
   0,				  /* tp_compare */
-  0,				  /* tp_repr */
+  infpy_repr,			  /* tp_repr */
   0,				  /* tp_as_number */
   0,				  /* tp_as_sequence */
   0,				  /* tp_as_mapping */
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index c2b40ff5352..61d3a158198 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -456,6 +456,21 @@ objfpy_add_separate_debug_file (PyObject *self, PyObject *args, PyObject *kw)
   Py_RETURN_NONE;
 }
 
+/* Implement repr() for gdb.Objfile.  */
+
+static PyObject *
+objfpy_repr (PyObject *self_)
+{
+  objfile_object *self = (objfile_object *) self_;
+  objfile *obj = self->objfile;
+
+  if (obj == nullptr)
+    return PyString_FromString ("<gdb.Objfile (invalid)>");
+
+  return PyString_FromFormat ("<gdb.Objfile filename=%s>",
+			      objfile_filename (obj));
+}
+
 /* Subroutine of gdbpy_lookup_objfile_by_build_id to simplify it.
    Return non-zero if STRING is a potentially valid build id.  */
 
@@ -709,7 +724,7 @@ PyTypeObject objfile_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  objfpy_repr,			  /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index a943d5f214e..ce3d7d63010 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -45,7 +45,7 @@ if ![runto_main] then {
 # Test basic gdb.Inferior attributes and methods.
 
 gdb_py_test_silent_cmd "python inferiors = gdb.inferiors ()" "get inferiors list" 1
-gdb_test "python print (inferiors)" "\\(<gdb.Inferior object at 0x\[\[:xdigit:\]\]+>,\\)" "verify inferiors list"
+gdb_test "python print (inferiors)" "\\(<gdb.Inferior num=1, pid=$decimal>,\\)" "verify inferiors list"
 gdb_py_test_silent_cmd "python i0 = inferiors\[0\]" "get first inferior" 0
 
 gdb_test "python print ('result = %s' % (i0 == inferiors\[0\]))" " = True" "test equality comparison (true)"
@@ -266,3 +266,15 @@ with_test_prefix "selected_inferior" {
     gdb_test "inferior 1" ".*" "switch back to first inferior"
     gdb_test_no_output "remove-inferiors 3" "remove second inferior"
 }
+
+# Test repr()/str()
+with_test_prefix "__repr__" {
+    gdb_test "add-inferior" "Added inferior 4" "add inferior 4"
+    gdb_py_test_silent_cmd "python infs = gdb.inferiors()" "get inferior list" 1
+    gdb_test "python print (infs\[0\])" "<gdb.Inferior num=1, pid=$decimal>"
+    gdb_test "python print (infs)" "\\\(<gdb.Inferior num=1, pid=$decimal>, <gdb.Inferior num=4, pid=$decimal>\\\)" \
+	"print all inferiors 1"
+    gdb_test_no_output "remove-inferiors 4"
+    gdb_test "python print (infs)" "\\\(<gdb.Inferior num=1, pid=$decimal>, <gdb.Inferior \\\(invalid\\\)>\\\)" \
+	"print all inferiors 2"
+}
diff --git a/gdb/testsuite/gdb.python/py-objfile.exp b/gdb/testsuite/gdb.python/py-objfile.exp
index 6e81750cfe9..240de29efd1 100644
--- a/gdb/testsuite/gdb.python/py-objfile.exp
+++ b/gdb/testsuite/gdb.python/py-objfile.exp
@@ -45,6 +45,8 @@ gdb_test "python print (objfile.filename)" "${testfile}" \
 gdb_test "python print (objfile.username)" "${testfile}" \
   "Get objfile user name"
 
+gdb_test "python print (objfile)" "<gdb.Objfile filename=[string_to_regexp ${binfile}]>"
+
 gdb_test_no_output "python dir(objfile)"
 
 gdb_test "python print (gdb.lookup_objfile (\"${testfile}\").filename)" \
@@ -149,3 +151,8 @@ if [remote_file host exists "${symlink_binary}"] {
     gdb_test "python print (gdb.lookup_objfile (\"${symlink_binary}\").filename)" \
 	"${testfile}" "gdb.lookup_objfile of symlinked binary"
 }
+
+# Test printing an Objfile object that is no longer valid.
+gdb_py_test_silent_cmd "python objfile = gdb.objfiles()\[0\]" "get first objfile" 1
+gdb_file_cmd ${binfile}
+gdb_test "python print(objfile)" "<gdb.Objfile \\\(invalid\\\)>"
\ No newline at end of file
diff --git a/gdb/testsuite/gdb.python/py-symtab.exp b/gdb/testsuite/gdb.python/py-symtab.exp
index f504362526f..4083b3b4475 100644
--- a/gdb/testsuite/gdb.python/py-symtab.exp
+++ b/gdb/testsuite/gdb.python/py-symtab.exp
@@ -66,7 +66,7 @@ gdb_test "python print (sal.is_valid())" "True" "test sal.is_valid"
 
 # Test symbol table.
 gdb_test "python print (symtab.filename)" ".*${py_symbol_c}" "test symtab.filename"
-gdb_test "python print (symtab.objfile)" "<gdb.Objfile object at ${hex}>" "test symtab.objfile"
+gdb_test "python print (symtab.objfile)" "<gdb.Objfile filename=${binfile}>" "test symtab.objfile"
 gdb_test "python print (symtab.fullname())" ".*${full_py_symbol_c}" "test symtab.fullname"
 gdb_test "python print (symtab.is_valid())" "True" "test symtab.is_valid()"
 gdb_test "python print (\"qq\" in global_symbols)" "True" "test qq in global symbols"
-- 
2.19.0

Re: [PATCH] python: Provide textual representation for Inferior and Objfile

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> This makes printing debug traces more difficult than it should be.  This
Simon> patch provides some repr() implementation for these two types (more to
Simon> come if people agree with the idea, but I want to test the water first).

I think it is a good idea.

Simon> In your opinion, does this require an announcement in NEWS and an entry
Simon> in the manual?  I don't think the output of repr() should be considered
Simon> stable, as we could change it over time to make it more useful.  Maybe
Simon> that should be documented?

That seems like a reasonable rule to me.  One idea would be to make this
a general rule for the gdb Python layer and document it in the "Basic
Python" node.

I could go either way about a NEWS entry.  It seems more like a
debugging convenience feature maybe?  Dunno.

The patch looks good.

Tom

Re: [PATCH] python: Provide textual representation for Inferior and Objfile

[Simon Marchi]

On 2018-09-12 18:07, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> This makes printing debug traces more difficult than it should 
> be.  This
> Simon> patch provides some repr() implementation for these two types 
> (more to
> Simon> come if people agree with the idea, but I want to test the water 
> first).
> 
> I think it is a good idea.
> 
> Simon> In your opinion, does this require an announcement in NEWS and 
> an entry
> Simon> in the manual?  I don't think the output of repr() should be 
> considered
> Simon> stable, as we could change it over time to make it more useful.  
> Maybe
> Simon> that should be documented?
> 
> That seems like a reasonable rule to me.  One idea would be to make 
> this
> a general rule for the gdb Python layer and document it in the "Basic
> Python" node.

Good idea, a blanket warning (for string repr of all object types) in 
there should be enough.  I'll do it that.

> I could go either way about a NEWS entry.  It seems more like a
> debugging convenience feature maybe?  Dunno.

That's what I think too.

Thanks,

Simon

[PATCH v2, doc] python: Provide textual representation for Inferior and Objfile

[Simon Marchi]

[New in v2: the doc part]

Printing a GDB Python object is notoriously not helpful:

>>> print(gdb.selected_inferior())
<gdb.Inferior object at 0x7fea59aed198>
>>> print(gdb.objfiles())
[<gdb.Objfile object at 0x7fea59b57c90>]

This makes printing debug traces more difficult than it should be.  This
patch provides some repr() implementation for these two types (more to
come if people agree with the idea, but I want to test the water first).
Here's the same example as above, but with this patch:

>>> print(gdb.selected_inferior())
<gdb.Inferior num=1>
>>> print(gdb.objfiles())
[<gdb.Objfile filename=/home/emaisin/build/binutils-gdb-gcc-git/gdb/test>]

I implemented repr rather than str, because when printing a list (or
another container I suppose), Python calls the repr method of the
elements.  This is useful when printing a list of inferiors or objfiles.
The print(gdb.objfiles()) above would not have worked if I had
implemented str.

I found this post useful to understand the difference between repr and
str:

  https://stackoverflow.com/questions/1436703/difference-between-str-and-repr

gdb/ChangeLog:

	* python/py-inferior.c (infpy_repr): New.
	(inferior_object_type): Register infpy_repr.
	* python/py-objfile.c (objfpy_repr): New.
	(objfile_object_type): Register objfpy_repr.

gdb/testsuite/ChangeLog:

	* gdb.python/py-inferior.exp: Test repr() of gdb.Inferior.
	* gdb.python/py-objfile.exp: Test repr() of gdb.Objfile.
	* gdb.python/py-symtab.exp: Update test printing an objfile.

gdb/doc/ChangeLog:

	* python.texi (Basic Python): Mention the string representation
	of GDB Python objects.
---
 gdb/doc/python.texi                      |  4 ++++
 gdb/python/py-inferior.c                 | 17 ++++++++++++++++-
 gdb/python/py-objfile.c                  | 17 ++++++++++++++++-
 gdb/testsuite/gdb.python/py-inferior.exp | 14 +++++++++++++-
 gdb/testsuite/gdb.python/py-objfile.exp  |  7 +++++++
 gdb/testsuite/gdb.python/py-symtab.exp   |  2 +-
 6 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 5e2ab76d2bc..1c70088acea 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -207,6 +207,10 @@ methods and classes added by @value{GDBN} are placed in this module.
 @value{GDBN} automatically @code{import}s the @code{gdb} module for
 use in all scripts evaluated by the @code{python} command.
 
+Some types of the @code{gdb} module come with a textual representation
+(accessible through @code{repr()} or @code{str()}).  These are offered for
+debugging purposes only, expect them to change over time.
+
 @findex gdb.PYTHONDIR
 @defvar gdb.PYTHONDIR
 A string containing the python directory (@pxref{Python}).
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 1cf37296973..56019bf9e05 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -860,6 +860,21 @@ infpy_thread_from_thread_handle (PyObject *self, PyObject *args, PyObject *kw)
   return result;
 }
 
+/* Implement repr() for gdb.Inferior.  */
+
+static PyObject *
+infpy_repr (PyObject *obj)
+{
+  inferior_object *self = (inferior_object *) obj;
+  inferior *inf = self->inferior;
+
+  if (inf == nullptr)
+    return PyString_FromString ("<gdb.Inferior (invalid)>");
+
+  return PyString_FromFormat ("<gdb.Inferior num=%d, pid=%d>",
+			      inf->num, inf->pid);
+}
+
 
 static void
 infpy_dealloc (PyObject *obj)
@@ -991,7 +1006,7 @@ PyTypeObject inferior_object_type =
   0,				  /* tp_getattr */
   0,				  /* tp_setattr */
   0,				  /* tp_compare */
-  0,				  /* tp_repr */
+  infpy_repr,			  /* tp_repr */
   0,				  /* tp_as_number */
   0,				  /* tp_as_sequence */
   0,				  /* tp_as_mapping */
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index c2b40ff5352..61d3a158198 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -456,6 +456,21 @@ objfpy_add_separate_debug_file (PyObject *self, PyObject *args, PyObject *kw)
   Py_RETURN_NONE;
 }
 
+/* Implement repr() for gdb.Objfile.  */
+
+static PyObject *
+objfpy_repr (PyObject *self_)
+{
+  objfile_object *self = (objfile_object *) self_;
+  objfile *obj = self->objfile;
+
+  if (obj == nullptr)
+    return PyString_FromString ("<gdb.Objfile (invalid)>");
+
+  return PyString_FromFormat ("<gdb.Objfile filename=%s>",
+			      objfile_filename (obj));
+}
+
 /* Subroutine of gdbpy_lookup_objfile_by_build_id to simplify it.
    Return non-zero if STRING is a potentially valid build id.  */
 
@@ -709,7 +724,7 @@ PyTypeObject objfile_object_type =
   0,				  /*tp_getattr*/
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
-  0,				  /*tp_repr*/
+  objfpy_repr,			  /*tp_repr*/
   0,				  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   0,				  /*tp_as_mapping*/
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 055dd87c6d8..077279f2f54 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -45,7 +45,7 @@ if ![runto_main] then {
 # Test basic gdb.Inferior attributes and methods.
 
 gdb_py_test_silent_cmd "python inferiors = gdb.inferiors ()" "get inferiors list" 1
-gdb_test "python print (inferiors)" "\\(<gdb.Inferior object at 0x\[\[:xdigit:\]\]+>,\\)" "verify inferiors list"
+gdb_test "python print (inferiors)" "\\(<gdb.Inferior num=1, pid=$decimal>,\\)" "verify inferiors list"
 gdb_py_test_silent_cmd "python i0 = inferiors\[0\]" "get first inferior" 0
 
 gdb_test "python print ('result = %s' % (i0 == inferiors\[0\]))" " = True" "test equality comparison (true)"
@@ -279,3 +279,15 @@ with_test_prefix "selected_inferior" {
     gdb_test "inferior 1" ".*" "switch back to first inferior"
     gdb_test_no_output "remove-inferiors 3" "remove second inferior"
 }
+
+# Test repr()/str()
+with_test_prefix "__repr__" {
+    gdb_test "add-inferior" "Added inferior 4" "add inferior 4"
+    gdb_py_test_silent_cmd "python infs = gdb.inferiors()" "get inferior list" 1
+    gdb_test "python print (infs\[0\])" "<gdb.Inferior num=1, pid=$decimal>"
+    gdb_test "python print (infs)" "\\\(<gdb.Inferior num=1, pid=$decimal>, <gdb.Inferior num=4, pid=$decimal>\\\)" \
+	"print all inferiors 1"
+    gdb_test_no_output "remove-inferiors 4"
+    gdb_test "python print (infs)" "\\\(<gdb.Inferior num=1, pid=$decimal>, <gdb.Inferior \\\(invalid\\\)>\\\)" \
+	"print all inferiors 2"
+}
diff --git a/gdb/testsuite/gdb.python/py-objfile.exp b/gdb/testsuite/gdb.python/py-objfile.exp
index 6e81750cfe9..240de29efd1 100644
--- a/gdb/testsuite/gdb.python/py-objfile.exp
+++ b/gdb/testsuite/gdb.python/py-objfile.exp
@@ -45,6 +45,8 @@ gdb_test "python print (objfile.filename)" "${testfile}" \
 gdb_test "python print (objfile.username)" "${testfile}" \
   "Get objfile user name"
 
+gdb_test "python print (objfile)" "<gdb.Objfile filename=[string_to_regexp ${binfile}]>"
+
 gdb_test_no_output "python dir(objfile)"
 
 gdb_test "python print (gdb.lookup_objfile (\"${testfile}\").filename)" \
@@ -149,3 +151,8 @@ if [remote_file host exists "${symlink_binary}"] {
     gdb_test "python print (gdb.lookup_objfile (\"${symlink_binary}\").filename)" \
 	"${testfile}" "gdb.lookup_objfile of symlinked binary"
 }
+
+# Test printing an Objfile object that is no longer valid.
+gdb_py_test_silent_cmd "python objfile = gdb.objfiles()\[0\]" "get first objfile" 1
+gdb_file_cmd ${binfile}
+gdb_test "python print(objfile)" "<gdb.Objfile \\\(invalid\\\)>"
\ No newline at end of file
diff --git a/gdb/testsuite/gdb.python/py-symtab.exp b/gdb/testsuite/gdb.python/py-symtab.exp
index f504362526f..4083b3b4475 100644
--- a/gdb/testsuite/gdb.python/py-symtab.exp
+++ b/gdb/testsuite/gdb.python/py-symtab.exp
@@ -66,7 +66,7 @@ gdb_test "python print (sal.is_valid())" "True" "test sal.is_valid"
 
 # Test symbol table.
 gdb_test "python print (symtab.filename)" ".*${py_symbol_c}" "test symtab.filename"
-gdb_test "python print (symtab.objfile)" "<gdb.Objfile object at ${hex}>" "test symtab.objfile"
+gdb_test "python print (symtab.objfile)" "<gdb.Objfile filename=${binfile}>" "test symtab.objfile"
 gdb_test "python print (symtab.fullname())" ".*${full_py_symbol_c}" "test symtab.fullname"
 gdb_test "python print (symtab.is_valid())" "True" "test symtab.is_valid()"
 gdb_test "python print (\"qq\" in global_symbols)" "True" "test qq in global symbols"
-- 
2.19.0

Re: [PATCH v2, doc] python: Provide textual representation for Inferior and Objfile

[Eli Zaretskii]

> From: Simon Marchi <simon.marchi@ericsson.com>
> CC: Simon Marchi <simon.marchi@ericsson.com>
> Date: Wed, 12 Sep 2018 18:50:01 -0400
> 
> [New in v2: the doc part]

Thanks.

> +Some types of the @code{gdb} module come with a textual representation
> +(accessible through @code{repr()} or @code{str()}).  These are offered for
> +debugging purposes only, expect them to change over time.

Please use @code{repr}, without the parens.  GNU Coding Standards
frown on using the latter to indicate a function:

     Please do not write '()' after a function name just to indicate it is
  a function.  'foo ()' is not a function, it is a function call with no
  arguments.

Otherwise OK.

Re: [PATCH v2, doc] python: Provide textual representation for Inferior and Objfile

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> +gdb_test "python print (inferiors)" "\\(<gdb.Inferior num=1, pid=$decimal>,\\)" "verify inferiors list"

Some of the new test lines run over 80 characters.

Other than that nit the patch looks good to me.

Tom

Re: [PATCH v2, doc] python: Provide textual representation for Inferior and Objfile

[Simon Marchi]

On 2018-09-13 10:36, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> +gdb_test "python print (inferiors)" "\\(<gdb.Inferior num=1,
> pid=$decimal>,\\)" "verify inferiors list"
> 
> Some of the new test lines run over 80 characters.

Most of them are easy enough to make fit in 80 chars, but one is still 
over, because it has a long match pattern string.  My understanding was 
that we were a bit more lenient on the line length for those gdb_test 
lines (splitting the patterns just to comply to that rule would just 
make them less readable).

> Other than that nit the patch looks good to me.

Thanks, I pushed it with those fixed as much as I can.  I also addressed 
Eli's comment about repr() and str().

Simon

Re: [PATCH v2, doc] python: Provide textual representation for Inferior and Objfile

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> On 2018-09-13 10:36, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>> 
Simon> +gdb_test "python print (inferiors)" "\\(<gdb.Inferior num=1,
>> pid=$decimal>,\\)" "verify inferiors list"
>> 
>> Some of the new test lines run over 80 characters.

Simon> Most of them are easy enough to make fit in 80 chars, but one is still
Simon> over, because it has a long match pattern string.  My understanding
Simon> was that we were a bit more lenient on the line length for those
Simon> gdb_test lines (splitting the patterns just to comply to that rule
Simon> would just make them less readable).

Yeah, I agree.

Tom