[GRASS-dev] [GRASS GIS] #2289: v.colors not working

[Huidae Cho]

I think in the long term it would be better to change the raster directory
structure such that raster and vector have the same mapset/(raster,
vector)/mapname/element path. Then we would be able to move some useful
element related functions from librast to libgis.

Huidae
On May 13, 2014 5:11 PM, "Glynn Clements" <glynn at gclements.plus.com> wrote:

>
> Huidae Cho wrote:
>
> > I agree. Maybe, Markus M has a better idea why we needed r60149?
>
> AFAICT, the reason for the logic change is so that it doesn't print a
> warning if the file simply doesn't exist.
>
> The previous version would only call G_find_file2() if the mapset
> couldn't otherwise be determined, i.e. the name was unqualified and no
> mapset was given. If the mapset could be determined either from a
> qualified name or a non-empty mapset parameter, a non-existent file
> wouldn't be detected until the open() call failed.
>
> The new version always calls G_find_file2() first, so open() should
> never be called for a non-existent file (or one which is inaccessible
> due to the current process lacking execute (search) permission on any
> ancestor directory). So a failed open() would indicate something less
> mundane than a missing file.
>
> I'm assuming that some code used the functions for a combined "check
> whether it exists, and if so, open it" function. If that's the case,
> warning about non-existent files would be a nuisance.
>
> I'm still not sure whether this behaviour is a good idea, or whether
> we should always generate the warning and require callers to perform
> an explicit G_find_* call if the file's existence is merely "useful"
> rather than either "required" or at least "expected".
>
> And none of this changes the fact that Vect_read_colors() was broken
> prior to r60225, and is still broken now.
>
> If a function has parameters which happen to be used as the various
> components of a filename, code shouldn't assume that they're just
> going to get glued together and that it doesn't matter about the
> individual pieces.
>
> It's still broken now because the first argument to Rast__read_colors
> is supposed to be an "element name", not two directory names glued
> together. That it happens to work as a coincidence of implementation
> details doesn't make it correct (in fact, "happens to work as a
> coincidence of implementation details" is pretty much the definition
> of a "hack").
>
> Given that the current prototype for Rast__read_colors() doesn't
> actually support reading colours for a vector map (because of the
> vector code using the <mapset>/vector/<map>/<element> layout rather
> than <mapset>/<element>/<map>), the correct solution is to add a new
> function which takes a FILE* and let Vlib deal with opening (and
> closing) the file.
>
> --
> Glynn Clements <glynn at gclements.plus.com>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20140513/a70ac888/attachment.html>