Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser fixes - bugs #5621 and #5636 #147

Closed
wants to merge 4 commits into from

Conversation

etiennesky
Copy link
Contributor

#5621 - make browser remove the file extensions when dragging and dropping layers into the canvas

This was discussed in the ML - patch adds file extensions only in browser but not in legend.
For better performance and /vsizip/ issues this requires a new member fileName(), because just stripping the extension does not work in these cases.
#5636 - browser loads .gz and .vrt files as both OGR and GDAL data items

Simple fix and new test for qgsdataitem.

@rouault
Copy link
Contributor

rouault commented May 24, 2012

Etienne,

For guessing if a .vrt is a GDAL VRT, I'd strongly recommand using GDALIdentifyDriver() instead of your custom logic. This will be less fragile (in case of format changes for example) and should be as fast (the file is opened only once, a buffer of 1024 bytes is read, and the drivers that implement Identify() only check that buffer to see if they are able to recognize it)

For OGR VRT, we don't have something equivalent for now. But perhaps using OGROpen() should not be that bad hopefully.

Even

@etiennesky
Copy link
Contributor Author

Even, thanks for the suggestion. Identify() is a good solution for the gdal provider (that I had overlooked, probably because OGR has no such mechanism), but I am concerned about speed issues with using OGROpen().

This is in the context of optimizing fast access to files, based on file extensions only (when /qgis/scanItemsInBrowser = 1 ). Perhaps if I tried to open the file using only the OGR VRT driver it would be fast?

@brushtyler
Copy link
Contributor

Etienne,

in ML Radim wrote: "It should be possible to get the file name from QgsDataItem::uri(). QgsDataItem is base for various data sources, most of them are not file based.".

The fileName() method is non-sense within the QgsDataItem class. E.g. database items have not filename.

Please, let's keep the discussion on ML with Radim and Martin.
Anyway even if have extensions in file browser seems very good to me, I'd prefer to keep things simple.

@etiennesky
Copy link
Contributor Author

Even, is there a way to call Identify() using only the VRT driver? Or is this only possible using the c++ api?

For the OGR case, I think I can call OGR_Dr_Open() using the OGR VRT driver and save lots of time.

@etiennesky
Copy link
Contributor Author

Even - if you are inclined please have a look at this new take, based on your comments:

etiennesky@e6cb3a1

Closing this request, will open a new one from this new take on .vrt and .gz, and another one for the name vs filename issue, pending more feedback from the list

@etiennesky etiennesky closed this May 25, 2012
@rouault
Copy link
Contributor

rouault commented May 25, 2012

etiennesky@e6cb3a1 looks fine to me. Just a little observation : instead of using CPLSetErrorHandler(), you could use CPLPushErrorHandler(CPLQuietErrorHandler) / CPLPopErrorHandler() that just modifies the error handler for the current thread. CPLSetErrorHandler() has process wide effects.

@etiennesky
Copy link
Contributor Author

ah ok, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants