[GRASS-dev] [bug #4368] (grass) g.parser: pass through --o

[Glynn Clements]

Radim Blazek wrote:

> > > Note that it is also possible to set region with GRASS_REGION
> > > environment variable.
> >
> > I wasn't aware of that feature. That would be simpler for running
> > commands with a specific region from gis.m.
> >
> > However, I don't think that the way it is currently implementated is a
> > good idea, as it overrides all saved regions as well as the current
> > region. I would suggest moving it to G_get_window(), so that programs
> > which use G__get_window() to read a specific named region aren't
> > affected.
> 
> Of course, I moved it to G_get_window(). But then g.region -p
> prints WIND file, not GRASS_REGION variable, I am not sure
> what it should. Probably it should print GRASS_REGION?

I would say so.

g.region doesn't use G_get_window() to get the current region, but
uses:

	if (G__get_window (&window, "", "WIND", G_mapset()) != NULL)
	{
		G_get_default_window (&window);
		G_put_window (&window);
	}

The logic here is that if the WIND file is invalid, the only way to
set it is to run g.region, so so g.region can't just just call
G_get_window() (which will generate a fatal error if the WIND file is
invalid).

Note that it does this before calling G_parser(), so that it can use
window.proj in the option definitions.

After G_parser() returns, it replaces the region with the default
region if the -d flag is used, or with the named region if region= is
used. Otherwise, the previously-obtained region is retained and
modified according to the other options.

There are a few possible solutions.

One is to obtain the initial region using G_get_default_window(). 
After G_parser() returns, if the -d flag isn't given, it would use
G_get_window() to replace this with the current region. That would
allow the use of "g.region -d" to fix a missing/invalid WIND file,
while causing GRASS_REGION and WIND_OVERRIDE to work normally.

The other is to explicitly test for GRASS_REGION and/or WIND_OVERRIDE
in g.region.

> > I've recently fixed a small number of modules which gratuitously used
> > G__get_window(..., "WIND") to get the current region.
> >
> > [Most of these did so in order to handle the case where the current
> > region was undefined, and use the default region instead. If that's
> > considered a good idea, it should be done in G_get_window() so that it
> > works for every module, not by a handful of specific modules.]
> 
> I think that undefined region is error and it should result in
> G_fatal_error().

Agreed.

But even if the fault-tolerant behaviour was desirable, it should be
implemented in the libraries, rather than a handful of modules doing
it themselves.

> > For future development, I'm wondering if it would be a good idea to
> > replace the WIND file with a GRASS variable (e.g. REGION) using the
> > same syntax as GRASS_REGION.
> 
> Do you mean GRASS mapset variable (in VAR)?

Well, I meant that G_get_window() would become use G_getenv("REGION")
rather than G__get_window(..., "WIND"). It wouldn't really matter if
it was a mapset or GISRC variable; the main point was not to store
state in a form which can only have a single instance. Actually, the
same issue applies to the VAR file; what uses that? Just QGIS?

If all "session state" goes into $GISRC, you can just create a new
$GISRC file for each operation, without worrying about conflicts
(providing that the operations only read the data; supporting
concurrent modification is a long way off).

> I am using WIND as test for mapset in QGIS, exists another way
> to test if a directory in location is a mapset?

You could test whether ../PERMANENT/DEFAULT_WIND exists. Or even just
whether ../PERMANENT exists.

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