[GRASS-dev] g.mapsets revision 30712

[Glynn Clements]

Martin Landa wrote:

> 2008/4/4, Markus Neteler OSGeo <neteler at osgeo.org>:
> >  >       r30712 | martinl | 2008-03-24 11:19:11 +0000 (Mon, 24 Mar 2008) | 2 lines
> >  >       g.mapsets: (cosmetics) message cleaning, some minor changes in manual,
> >  > tcl/tk-related code removed
> Glynn:
> >  > Presumably g.mapsets.tcl should be removed, now that it is no longer
> >  > used.
> 
> > The CMD line version of g.mapsets is lacking a parameter to
> >  remove (multiple) mapsets from the search path.
> >
> >  This should be added before removing the Tcl version.
> >  I tried but got lost in the tokenizer.
> 
> try the attached patch (for review), Martin

I should probably have read this message before going ahead and making
my own patch as soon as I read Markus' message.

FWIW:

> +    opt3->key        = "delmapset" ;

I was going to call the option that, then decided that "remove" is
more accurate than "delete".

Also, I'm not sure what the policy is (or even if there is a policy)
on using abbreviations in option names. For a single word, it's normal
to used the complete word, as option names can be abbreviated. For
multiple words, it's more awkward.

If it wasn't for the fact that addmapset= already exists, it would
have been better to just have add= and remove= (or delete=). The
"mapset" in the option names is redundant, IMHO.

In the end, I went for removemapset=. For interactive command-line
use, I would expect the user to abbreviate it to "rem=" anyhow.

> +	tokens = G_tokenize (opt3->answer, ",");

This isn't necessary. When ->multiple == YES, the parser will split
the ->answer string into components which are stored in the ->answers
array (see the code for the other options).

> +		    G_verbose_message(_("Mapset <%s> removed from search path"),
> +				      oldname);

If this is desirable, should the addmapset= option have something
similar?

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