>>>>> Hamish  <[EMAIL PROTECTED]> 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.)

_______________________________________________
grass-dev mailing list
[email protected]
http://lists.osgeo.org/mailman/listinfo/grass-dev

Reply via email to