[GRASS-dev] g.mlist as C implementation?

[Glynn Clements]

Hamish wrote:

> > > I will have a look at merging Martin, Markus and Glynn's suggestions for
> > > a flat output g.list flag. As the feature is needed for parsing, I'd
> > > rather do it as a flag vs. a ncols= option which retains the text and
> > > fancy format mapset underlines. I've never really known what the "g"
> > > stands for, but as we use it a lot for parsable output, the flag will
> > > be '-g' here too.
> > > 
> > > so it would look like:
> ...
> > > G63> g.list -gf rast
> > > map1 at user1:
> > > map2 at user1:This map has a title
> > > map3 at user1:
> > > map_a at PERMANENT:Raw x,y,z data binned into a raster grid by cell min
> > > map_b at PERMANENT:Land Use
> > > map_c at PERMANENT:
> > 
> > According to G_legal_filename(), ":" is legal in map names (that won't
> > work on Windows):
> > 
> > 	if (*s == '/' || *s == '"' || *s == '\'' || *s <= ' ' || 
> >             *s == '@' || *s == ',' || *s == '=' || *s == '*' || *s > 0176) {
> > 		fprintf(stderr, _("Illegal filename. Character <%c> not allowed.\n"), *s);
> > 
> > I'd suggest using a comma for the field separator.
> 
> I'm not crazy about commas as field separators as there's a good chance that
> the map title has some in it and a poorly written script would easily do
> something like "cut -d, -f2" and chop it in half. Same goes for using a single
> ' ' space.

Anything can appear in the map title. Scripts which process "full"
output will have to be written correctly (i.e. break at the first
occurrence) whichever separator is used.

> My two preferred choices would be : or |.
>  ":" mimics what we use for deg:min:sec separators and makes some contextual   
> 
>    sense as a sep.
>  "|" mimics what we use for vector attribute column separators.
> 
> (trying to reuse ideas wherever possible for consistency's sake)

Of those two, | is less likely to appear in titles. Personally, I'd
choose the most common character (probably comma) as a separator so
that bugs in scripts show up sooner rather than later.

> We could add : and | to the G_legal_filename() blacklist, even though that
> could hurt backwards compatibility if the fn is used to test ,,old_map names.
> Esp. that, as you point out, calling a map "C:" is problematic on Windows.
> (my vote it is to do it after making sure that g.rename doesn't call
> G_legal_filename() for the old map name)

I suggest only using G_legal_filename for new files/mapsets/etc. For
existing files, just try to find the named object; if the name was
never legal, then it won't exist.

Also, : isn't valid anywhere in a filename on Windows (or on Windows
filesystems generally; Linux can use FAT and SMB filesystems).

> Using "=" for the sep seems inappropriate as
> - it looks funny in this context, and 
> - "map at mapset=Some Title" wouldn't be reasonably used as eval `shell variable`.
> 
> If all options are deemed too ugly I guess we could use the old \t standby.

Again, \t is valid in titles.

Actually, I'll change my preference for using a comma as the separator
to a preference for using a space. That's almost certain to occur in
titles (but cannot occur in names), so that will almost guarantee that
any scripts handle the separator correctly.

> Having now looked at the g.list code all I can say is yuk. There's about 7
> convoluted layers of abstraction and G_spawn()ing to executables with MINGW32
> #ifdefs and pipe checking and "more" paging thrown in when AFAICT there really
> only needs to be 2 or 3 layers of clean C functions.
> 
> general/manage/cmd/list.c
> general/manage/lister/{cell|vector}  -> $GISBASE/etc/lister/*.exe
> general/manage/lib/do_list.c
> lib/gis/list.c
> lib/gis/ls.c
> 
> seem like they could all be combined into just
>   general/manage/cmd/list.c  +  lib/gis/ls.c

AFAICT, the "lister" idea is meant to ensure that neither g.list nor
libgis needs to be updated if a new type is added to etc/element_list. 
IOW, the set of possible element types isn't hard-coded anywhere.

But, yes, the whole setup generally is a mess.

> My idea was to do a new simple G_list_element_flat() fn in lib/gis/list.c
> which just creates a G__ls() list then loops through and fprintf(out,)s the
> name, optionally the mapset, and optionally the sep+map title*, then closing
> '\n'.
> 
> [*] title only makes sense if element is cell or vector, silently set to ""
> otherwise.

libgis doesn't (and cannot) depend upon the vector libraries, so a
libgis function can't get the title for a vector map.

> I'm not really sure, Martin's patch is fine but I don't see any way to extent
> it to append "@mapset" and "[sep]Map Title" as G_ls_format() goes right to the
> stream. My idea above kind of suffers from the same problem- It seems a bit
> wrong to have the same function be so low level that it calls {G_}ls and so
> high level that it evals the result to query the map title with. It's nice to
> have a deep lib fn do the list but the dump to stdout has to happen after the
> title query and appending.
> 
> 
> I would like to avoid having the new fn rely on any of the lister stuff or
> pagers if at all possible. Just send result to FILE*out. (maybe I'm just not
> good enough in C to write my own custom function to use for the lister arg and
> the solution really isn't that hard, just follow the 'g.list -f' example?)

I'd be inclined to suggest that most of G_list_element belongs in
g.list rather than in libgis. However, a number of modules actually
use it, so we're probably stuck with keeping it in libgis until 7.x.

> So perhaps we could use this opportunity to simplify this part of the code?
>   (about here's where I cry oceanographer not software engineer ;)
> 
> Implementing 'g.list -g' SHOULD just be a 5 minute job..... but it ain't.

"g.list -g" probably is a 5-minute job. "g.list -gf" is likely to be
rather more work, due to the need to accomodate new types in
etc/element_list without re-compilation.

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