[GRASS-dev] [GRASS GIS] #32: r.what: shouldn't use static buffers for the inputs

[Ivan Shmakov]

>>>>> Hamish  <hamish_b at yahoo.com> writes:

 >> Both of the modules currently have some minor deficiencies, e. g.:

 > ...

 >> * it's not possible to specify a different field separator string
 >> (`fs') with `r.what' (while it's possible with `v.db.select'.)

 > That one's trival. done in svn/trunk.
 > http://trac.osgeo.org/grass/changeset/30112

	Thanks!

 > @@ -144,4 +147,16 @@
 >    projection = G_projection();

	BTW, did anyone notice that the `projection' variable is unused?

	Apparently, the only side effect of this line is to have
	G__init_window () called (via G_get_set_window ().)  The
	questions are:

	* is this call necessary in `r.what.rast'?

	* shouldn't this side effect of G_projection () and
	  G_get_set_window () be documented?

 > +    /* see v.in.ascii for a better solution */
 > +    if (opt_fs->answer != NULL) {
 > +        if (strcmp(opt_fs->answer, "space") == 0)
 > +            fs = ' ';
 > +        else if (strcmp(opt_fs->answer, "tab") == 0)
 > +            fs = '\t';
 > +        else if (strcmp(opt_fs->answer, "\\t") == 0)
 > +            fs = '\t';
 > +        else
 > +            fs = opt_fs->answer[0];
 > +    }
 > +

	Oh, the piece of code like this is a sure candidate for a
	library function! (taking into account the number of times it
	appears in the GRASS source.)

 >    withcats = 0;
 >    nfiles = 0;

 > I did it as %c like r.cats, other modules are more liberal and allow
 > %s.  Shrug.

	I have no use for %s just now, but there should probably be a
	GRASS-wide policy on using either %c or %s for `fs'.  (In order
	for the user to learn this exactly once.)