[GRASS-dev] g.mlist: doesn't show a single raster?

[Ivan_Shmakov1]

  I've compiled a recent SVN (r30272) GRASS, and now getting a
  strange behaviour from `g.mlist' and `g.list'. E. g.:

GRASS 6.3.svn (modis-sin-i):~ > g.mlist type=rast mapset=ivan
GRASS 6.3.svn (modis-sin-i):~ >

  This works well with GRASS 6.2.3 from Debian (though it takes
  noticeable time to complete):

GRASS 6.2.3 (modis-sin-i):~ > g.mlist type=rast mapset=ivan
... 3106 lines skipped...
2006-08-brdf.a.albedo.altay.8.qa-mask
2006-08-brdf.a.albedo.altay.9.qa-mask
foo
GRASS 6.2.3 (modis-sin-i):~ >

  Also (though it presumably is a different issue):

GRASS 6.3.svn (modis-sin-i):~ > gdb g.list
...
(gdb) set args type=rast mapset=ivan
Breakpoint 2, G_ls_format (list=0x807f4a8, num_items=3109, perline=0,
    stream=0x8056ec0) at ls.c:157
157 perline = screen_width / (max_len + 1);
(gdb) print screen_width
$7 = 80
(gdb) print max_len
$8 = 110
(gdb)

  NB: the `max_len >= screen_width' case isn't handled properly!

(gdb) next
161 field_width = screen_width / perline;
(gdb) print perline
$9 = 0
(gdb) next

Program received signal SIGFPE, Arithmetic exception.
0xf7e821a0 in G_ls_format (list=0x807f4a8, num_items=3109, perline=0,
    stream=0x8056ec0) at ls.c:161
161 field_width = screen_width / perline;
(gdb)

[Ivan_Shmakov1]

Ivan Shmakov <ivan@theory.asu.ru> writes:

[...]

> Also (though it presumably is a different issue):

[...]

> NB: the `max_len >= screen_width' case isn't handled properly!

[...]

  I stand corrected, it's completely the same issue, as long as
  `g.mlist' is implemented on top of `g.list'.

  Please consider the following patch.

lib/gis/ls.c (G_ls_format): Handle the case when the screen width is
less or equal to the maximum length of the name to be output; don't pad
the names with spaces when printing in 1-column.
(nntp://news.gmane.org/gmane.comp.gis.grass.devel/25019)

diff --git a/lib/gis/ls.c b/lib/gis/ls.c
index 44e02bd..c0cf4a5 100644
--- a/lib/gis/ls.c
+++ b/lib/gis/ls.c
@@ -155,10 +155,13 @@ void G_ls_format(const char **list, int num_items, int perline, FILE *stream)
                       /* Auto-fit the number of items that will
                        * fit per line (+1 because of space after item) */
         perline = screen_width / (max_len + 1);
+ if (perline < 1) perline = 1;
     }
    
     /* Field width to accomodate longest filename */
- field_width = screen_width / perline;
+ /* NB: don't pad the names with spaces when printing one name per
+ * line */
+ field_width = (perline > 1) ? (screen_width / perline) : 0;
     /* Longest column height (i.e. num_items <= perline * column_height) */
     column_height = (num_items / perline) + ((num_items % perline) > 0);

[Ivan_Shmakov1]

Ivan Shmakov <ivan@theory.asu.ru> writes:

[...]

> Please consider the following patch.

> lib/gis/ls.c (G_ls_format): Handle the case when the screen width is
> less or equal to the maximum length of the name to be output; don't
> pad the names with spaces when printing in 1-column.
> (nntp://news.gmane.org/gmane.comp.gis.grass.devel/25019)

[...]

  Since there were no objections, and the issue is rather major,
  I've commited the ``fix'' part of the change. As for the
  padding problem, I believe that a more generic solution should
  be implemented.

http://trac.osgeo.org/grass/changeset/30281

[Ivan_Shmakov1]

Ivan Shmakov <ivan@theory.asu.ru> writes:

[...]

> As for the padding problem, I believe that a more generic solution
> should be implemented.

  Such as the following (rather obscure, though actually obvious)
  one.

  BTW, GNU `ls' seems to produce quite nice columnar lists. In
  particular, the columns are allowed to be of different width.
  May the respective code be borrowed for G_ls_format ()?

diff --git a/lib/gis/ls.c b/lib/gis/ls.c
index 3ef14e9..b556212 100644
--- a/lib/gis/ls.c
+++ b/lib/gis/ls.c
@@ -124,7 +124,7 @@ void G_ls(const char *dir, FILE *stream)

void G_ls_format(const char **list, int num_items, int perline, FILE *stream)
{
- int i, j;
+ int i;

     int field_width, column_height;
     int screen_width = 80; /* Default width of 80 columns */
@@ -163,20 +163,25 @@ void G_ls_format(const char **list, int num_items, int perline, FILE *stream)
     /* Longest column height (i.e. num_items <= perline * column_height) */
     column_height = (num_items / perline) + ((num_items % perline) > 0);

- for (i=0; i < column_height; i++)
- {
- for (j=0; j < perline; j++)
- {
- int cur = j * column_height + i;
-
- if (cur >= num_items)
- continue; /* No more items to print in this row */
-
- /* Print filenames in left-justified fixed-width fields */
- fprintf(stream, "%-*s", field_width, list[cur]);
- }
- fprintf(stream, "\n");
- }
+ {
+ const int max
+ = num_items + column_height - (num_items % column_height);
+ const char **next;
+
+ for (i = 1, next = list; i <= num_items; i++) {
+ const char **cur = next;
+
+ next += column_height;
+ if (next >= list + num_items) {
+ /* the next item has to be on the other line */
+ next -= (max - 1
+ - (next < list + max ? column_height : 0));
+ fprintf (stream, "%s\n", *cur);
+ } else {
+ fprintf (stream, "%-*s", field_width, *cur);
+ }
+ }
+ }
    
     return;
}

[Ivan_Shmakov1]

Ivan Shmakov <ivan@theory.asu.ru> writes:
Ivan Shmakov <ivan@theory.asu.ru> writes:

IS> [...]

>> As for the padding problem, I believe that a more generic solution
>> should be implemented.

> Such as the following (rather obscure, though actually obvious)
> one.

[...]

  Since there were no objections, and since the extra whitespace
  may trigger extra linebreaks in the W32 ``console'' window:

$ g.list
bar baz

foo qux

...
$

  I've now committed the change:

http://trac.osgeo.org/grass/changeset/30423

[Paul_Kelly]

On Mon, 3 Mar 2008, Ivan Shmakov wrote:

Ivan Shmakov <ivan@theory.asu.ru> writes:

IS> [...]

>> As for the padding problem, I believe that a more generic solution
>> should be implemented.

> Such as the following (rather obscure, though actually obvious)
> one.

[...]

  Since there were no objections, and since the extra whitespace
  may trigger extra linebreaks in the W32 ``console'' window:

$ g.list
bar baz

foo qux

...
$

  I've now committed the change:

Thanks Ivan. The orginal code was mine (a "DIY" solution), and I was aware of the problem with the ugly linebreaks on Windows, but hadn't yet investigated it further. FWIW I thought no space after the last item on a line, rather than no no space when there is only one item on a line, was the obvious course of action to pursue, but I'm sure the code from GNU ls achieves the same even better than hacking my code would.

Paul

[Ivan_Shmakov1]

Paul Kelly <paul-grass@stjohnspoint.co.uk> writes:

>>>> As for the padding problem, I believe that a more generic
>>>> should be implemented.

>>> Such as the following (rather obscure, though actually obvious)
>>> one.

[...]

>> I've now committed the change:

> Thanks Ivan. The orginal code was mine (a "DIY" solution), and I was
> aware of the problem with the ugly linebreaks on Windows, but hadn't
> yet investigated it further. FWIW I thought no space after the last
> item on a line, rather than no no space when there is only one item
> on a line, was the obvious course of action to pursue,

  The committed change eliminates extra whitespace for whatever
  number of items on a line. The change I've suggested formely
  was, OTOH, much simpler:

     /* Field width to accomodate longest filename */
- field_width = screen_width / perline;
+ /* NB: don't pad the names with spaces when printing one name per
+ * line */
+ field_width = (perline > 1) ? (screen_width / perline) : 0;

  (Please note that `%0s' is identical to `%s' as far as printf ()
  is concerned.)

> but I'm sure the code from GNU ls achieves the same even better than
> hacking my code would.

  I didn't borrow the code from GNU ls as of yet (the piece was
  just too big for me to comprehend at once), and thus I'm solely
  responsible for the hack I've committed.

  Using the GNU ls code would be preferred, since it allows for
  variable length columns, like:

$ g.list ...
...
Bar Baz Foo Qux
a--name a-long-name an-even-longer-name-making-this-column-the-widest
$

  But actually, I think that a much needed change is to add both a
  ``single column'' and a ``no decoration'' mode for `g.list'.
  This would somewhat simplify `g.mlist', and would allow for
  iterating over the `g.list' result directly from the Shell:

$ g.list --no-decoration -1 ... \
      | (while read i ; do ... ; done)