[GRASS-dev] g.list -a

Martin Landa wrote:

[snip]

> > 2) It seems better to add the type 'all' to g.list manually
> > (especially because of the requested type parameter). I am not sure
> > how to do it in an elegant way (see the patch) - my approach seems to
> > be very ugly.
>
> I would abandon the approach of having a separate "parse" function in
> favour of iterating over element->answers and listing each type as
> it's encountered.

well, I have tried to simplify the patch. Functions parse() &
do_list() are now called for each element->answers[i]. I hope it is
better now.

+ if (G_parser(argc, argv))
+ {
+ fprintf (stderr, _("\nWhere %s is one of:\n"), element->key);
+ show_elements();
+ exit(EXIT_FAILURE);
+ }

If G_parser() fails, the program should just call exit(EXIT_FAILURE);
G_parser() will print an error message detailing why it failed.

> lib/do_list.c doesn't need to change; just have cmd/list.c call
> do_list() once for each type.

I think that fn do_list() have to be rewritten because of the -f flags
(calling of execl() fn) ?

No; do_list() can remain untouched, while the handling of -f should
remain essentially the same, except that it should use G_spawn()
instead of execl().

--
Glynn Clements <glynn@gclements.plus.com>

Hi Glynn,

2007/1/12, Glynn Clements <glynn@gclements.plus.com>:

[snip]

+ if (G_parser(argc, argv))
+ {
+ fprintf (stderr, _("\nWhere %s is one of:\n"), element->key);
+ show_elements();
+ exit(EXIT_FAILURE);
+ }

If G_parser() fails, the program should just call exit(EXIT_FAILURE);
G_parser() will print an error message detailing why it failed.

Yes, but the function show_elements() provides the user additional
information why it failed. Would you like to remove it (maybe I have
missed something)?

Where type is one of:
  rast (raster files)
  rast3d (raster3D files)
  vect (binary vector files)
  oldvect (old (GRASS 5.0) binary vector files)
  asciivect (ascii vector files)
  icon (paint icon files)
  labels (paint label files)
  sites (site list files)
  region (region definition files)
  region3d (region3D definition files)
  group (imagery group files)
  3dview (3D view parameters)

> > lib/do_list.c doesn't need to change; just have cmd/list.c call
> > do_list() once for each type.
>
> I think that fn do_list() have to be rewritten because of the -f flags
> (calling of execl() fn) ?

No; do_list() can remain untouched, while the handling of -f should
remain essentially the same, except that it should use G_spawn()
instead of execl().

Ops, you are right. Maybe I need study GRASS API more deeply:-) Thanks
for pointing to the G_spawn! Now it works.

Regards, Martin

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

(attachments)

g_list_all-6.diff.gz (2.11 KB)

Martin Landa wrote:

> + if (G_parser(argc, argv))
> + {
> + fprintf (stderr, _("\nWhere %s is one of:\n"), element->key);
> + show_elements();
> + exit(EXIT_FAILURE);
> + }
>
> If G_parser() fails, the program should just call exit(EXIT_FAILURE);
> G_parser() will print an error message detailing why it failed.

Yes, but the function show_elements() provides the user additional
information why it failed. Would you like to remove it (maybe I have
missed something)?

Where type is one of:
  rast (raster files)
  rast3d (raster3D files)
  vect (binary vector files)
  oldvect (old (GRASS 5.0) binary vector files)
  asciivect (ascii vector files)
  icon (paint icon files)
  labels (paint label files)
  sites (site list files)
  region (region definition files)
  region3d (region3D definition files)
  group (imagery group files)
  3dview (3D view parameters)

The built-in error handling will display the following message:

Error: value <foo> out of range for parameter <type>
       Legal range: rast,rast3d,vect,oldvect,asciivect,icon,labels,sites,region,region3d,group,3dview

IMHO, this is sufficient. Note that the type descriptions *aren't*
shown if you use "g.list help", only if there is a parsing error
(which isn't limited to an invalid value for the type= option).

OTOH, I'm not really that bothered about this issue; assuming that it
compiles okay, I'll commit your latest version.

> > > lib/do_list.c doesn't need to change; just have cmd/list.c call
> > > do_list() once for each type.
> >
> > I think that fn do_list() have to be rewritten because of the -f flags
> > (calling of execl() fn) ?
>
> No; do_list() can remain untouched, while the handling of -f should
> remain essentially the same, except that it should use G_spawn()
> instead of execl().

Ops, you are right. Maybe I need study GRASS API more deeply:-) Thanks
for pointing to the G_spawn! Now it works.

FWIW, I've added a native Windows version of G_spawn(), although I
haven't had chance to test it yet.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn,

I committed changes to CVS.

Once more thanks a lot for the help and valuable feedback.

Martin

2007/1/13, Glynn Clements <glynn@gclements.plus.com>:

Martin Landa wrote:

> > + if (G_parser(argc, argv))
> > + {
> > + fprintf (stderr, _("\nWhere %s is one of:\n"), element->key);
> > + show_elements();
> > + exit(EXIT_FAILURE);
> > + }
> >
> > If G_parser() fails, the program should just call exit(EXIT_FAILURE);
> > G_parser() will print an error message detailing why it failed.
>
> Yes, but the function show_elements() provides the user additional
> information why it failed. Would you like to remove it (maybe I have
> missed something)?
>
> Where type is one of:
> rast (raster files)
> rast3d (raster3D files)
> vect (binary vector files)
> oldvect (old (GRASS 5.0) binary vector files)
> asciivect (ascii vector files)
> icon (paint icon files)
> labels (paint label files)
> sites (site list files)
> region (region definition files)
> region3d (region3D definition files)
> group (imagery group files)
> 3dview (3D view parameters)

The built-in error handling will display the following message:

Error: value <foo> out of range for parameter <type>
       Legal range: rast,rast3d,vect,oldvect,asciivect,icon,labels,sites,region,region3d,group,3dview

IMHO, this is sufficient. Note that the type descriptions *aren't*
shown if you use "g.list help", only if there is a parsing error
(which isn't limited to an invalid value for the type= option).

OTOH, I'm not really that bothered about this issue; assuming that it
compiles okay, I'll commit your latest version.

> > > > lib/do_list.c doesn't need to change; just have cmd/list.c call
> > > > do_list() once for each type.
> > >
> > > I think that fn do_list() have to be rewritten because of the -f flags
> > > (calling of execl() fn) ?
> >
> > No; do_list() can remain untouched, while the handling of -f should
> > remain essentially the same, except that it should use G_spawn()
> > instead of execl().
>
> Ops, you are right. Maybe I need study GRASS API more deeply:-) Thanks
> for pointing to the G_spawn! Now it works.

FWIW, I've added a native Windows version of G_spawn(), although I
haven't had chance to test it yet.

--
Glynn Clements <glynn@gclements.plus.com>

--
Martin Landa <landa.martin@gmail.com> * http://gama.fsv.cvut.cz/~landa *

Martin Landa wrote:

I committed changes to CVS.

I've changed it to use G_spawn() rather than G_spawn_ex(), as it
doesn't need any of the advanced features provided by the latter, and
the former has a native Windows implementation.

Also, I've changed G_spawn[_ex] so that the argument list starts with
argv[0] rather than argv[1], for compatibility with execl() and
_spawnl(), and fixed a bug in G_spawn() (the terminating NULL pointer
wasn't being added to the array).

--
Glynn Clements <glynn@gclements.plus.com>