[GRASS-dev] r45050: v.info: -r/-m/-t -> shell param; GEM; parser.c

[Hamish]

Hamish:
> > re. https://trac.osgeo.org/grass/changeset/45050
...
> > what's the justification for introducing non-standard
> > behavior into v.info?
Martin: 
> are you referring to GRASS6?

both 6 and 7.

> If so, why don't change some "standard" or "conventions" in
> GRASS7? ;-)

because there's nothing really wrong with the old way?
If there seem to many buttons in the GUI for a module,
maybe the GUI module creation code should try for two columns
or something? personally I am not too overwhelmed by the
v.info and g.region 'Print' tabs, IMO they are fine and easy.

 
> I remember discussion about number of flags of g.region.
> There was suggestion to reduce number of flags as I have
> done for v.info.

it wasn't just discussion, it was committed and became a horrible
mess for 2-3 months before it got partially reverted/cleaned up,
and maybe now is still not as clean and clear as it once was.
IIRC it was having a single -g flag which changed the behavior
of all the other print flags. partly I'm trying to avoid that
mess from happening again. a good idea perhaps, but it just
didn't work in practice and made the entire GIS harder to use
because it introduced a new paradigm.


> The CLI follows clear design (group related flags to the one
> option). In the case of v.info -r/-m(and new -g) to
> `shell=basic,region,topo`.

no, what I mean is that for something like 25-100 popular modules,
the convention is to use -g to print shell style, -r to print
range etc. mostly only modules which print reports or dump
topology or so will use an option=print for that, but they are
quite the exception. the convention I am talking about is that
flags are used to print something and exit. If you use multiple
flags and you get both e.g. range and bounds. ('r.info -rg')
If you know this, or learn it for 2-3 modules, you magically
know how to use another 22-97 modules which act in the same way.


> GUI is also much more intuitive in the result (try it out).

It is a matter of taste and I don't find that way more intuitive,
but for the sake of argument say it was so. That's for individual
modules, but it breaks with how all the other modules act printing
shell style, so makes the entire GIS suite less consistent. Which
harms learning both the CLI /and/ the GUI, even if it's more
subtle in the GUI, the inconsistency is still there.

Also having it as multiple=yes makes a bit of a mess because 
then you can't use a pull down menu for that, which leads to
typos and wasted keystrokes/time. and well, I'll be honest:
personally I really like stacking the flags on the command line.
it's quite an efficient and clean way to write scripts.


> Same can be done for g.region, which ended up with extremely
> large number of flags. The goal is not to break conventions,
> just to make interface of GRASS modules more intuitive for the
> end-user. Of course it's question for discussion.
> 
> > (messing up the modules' CLIs to improve GUI cosmetics
> > makes me rather grumpy..)
> 
> In which way the CLI of v.info has been messed up?

- the convention of passing letter flags to print a list of
things you want quickly and easily is now broken.
- the module now needs a lot more keystrokes to accomplish
the same thing.


> > also, re. https://trac.osgeo.org/grass/changeset/45039,
> > is GEM fundamentally broken on WinGrass or is it buggy
...
> tools.c: In function `dump_plain':
> tools.c:572: error: implicit declaration of function
> `mkstemp'

somewhat both buggy & UNIXisms:
 https://trac.osgeo.org/grass/ticket/930#comment:6

again, I don't really mind GEM being switched off in WinGrass,
just that the disabling of large features should come with more
notification. anyway moving on,


...
> > it you got the usage wrong, and you didn't use --quiet, the
> > obvious thing you'll do next is to look up the correct one...
> > --verbose to get at it is not obvious, so you waste time
> > finding the right manual. theoretically the GUIs should
> > never get the usage wrong as they get it from the
> --interface-description.
> 
> OK, you are right that calling G_usage() should not be
> related the verbosity level.

I didn't say that. if it is run by default, --quiet should avoid
it. But the suggestion to use '--help' instead of G_usage()
is good & covers both of our concerns I think.


> I was always embarrassed with G_usage().

I've learned to accept my many typos without embarrassment :-)
if you type fast enough people don't notice. :-)
public thinkos are the embarrassing ones..

> $ r.info input=x --v
> Sorry, <input> is not a valid parameter
> ERROR: Required parameter <map> not set:
>         (Name of raster map)
[...]
> You need to scroll up to find out what is wrong. Extremely
> annoying.

much less annoying than having to look up the manual (if you
didn't know about --help). and it is slightly less annoying to
spin the mouse wheel than having to retype 'g.module --help'.

> Now it ends up with clear message
> 
> $ r.info input=x
> Sorry, <input> is not a valid parameter
> ERROR: Required parameter <map> not set:
>         (Name of raster map)
> 
> You get clear info what is wrong.

*only* if the thing you got wrong was a required option. if it
was clean=toolname spelt wrong you'd have to look it up. grass
is just too big to remember all of these things even after you're
comfortable on the command line. I'm often looking up the usage
for modules I wrote myself when I misremember them.. (eg was it
'mean' or 'average'? 'm' or 'meter'?)


fwiw ISTR Radim wrote up a clear doc of when map= should be used
and when input= should be used, and it made a lot of sense. I
vaguely remember it was outside of the main doc places, I'll
try to find it.


> Hm, about the GUI, what about launching commands from Layer
> Manager command line? Easy to make a mistake.

same issues. if you made a mistake the next logical thing is
to look up what the correct way is. If that's already done for
you, great, you can solve the problem faster.


[...]

Index: grass/branches/releasebranch_6_4/lib/gis/parser.c
===================================================================
--- a/grass/branches/releasebranch_6_4/lib/gis/parser.c
+++ b/grass/branches/releasebranch_6_4/lib/gis/parser.c
@@ -632,16 +632,16 @@
         Opt->key = "input";
         Opt->type = TYPE_STRING;
-        Opt->key_desc = "name";
+        Opt->key_desc = "path";
         Opt->required = YES;
         Opt->gisprompt = "old_file,file,input";
-        Opt->description = _("Name of input file");
+        Opt->description = _("Path to input file");
         break;
     case G_OPT_F_OUTPUT:
         Opt->key = "output";
         Opt->type = TYPE_STRING;
-        Opt->key_desc = "name";
+        Opt->key_desc = "path";
         Opt->required = YES;
         Opt->gisprompt = "new_file,file,output";
-        Opt->description = _("Name for output file");
+        Opt->description = _("Path for output file");
         break;
     case G_OPT_F_SEP:

Hamish:
> > and fwiw this one I think makes things more confusing,
> > as it implies `dirname` (vs `basename`), when the correct
> > usage is like [/path/to/]filename.ext.
Martin:
> Sorry I don't understand, this parameter takes path to the
> file (relative or absolute)...

it takes both the path and the file name. the path is optional
while the file name is not. The change makes it seem like the
path is required but the filename is optional. in absence of
experience, many new users will take these words literally and
become very confused.

Partly I complain because we have been very loose with "bug fixes
only in the 6.4 branch"; especially parser.c and gisdefs.h need a
very good reason to touch at this point. any change to them is
dangerous.


cheers & thanks,
Hamish