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

"type" option passed to advanced mapnik options for layer are not preserved #776

Open
springmeyer opened this issue Oct 10, 2011 · 4 comments

Comments

@springmeyer
Copy link
Member

This occurs if you use the special keyword of type.

For instance with the default project shapefile you can pass:

type="shape"

and the project continues working perfectly, but after saving the custom options string is not preserved.

The option is clearly being passed through to mapnik, as it should be, as `type="foo" will lead to an error like:

Error: Could not create datasource. No plugin found for type 'foo' (searched in: /usr/local/lib/mapnik2/input)

This ability to pass type is excellent - it means that the user can override the datasource plugin that mapnik uses for reading data. A developer usecase for this could be that a debug version of a plugin should be used (and has a different name), a more recent build should be used (https://github.com/springmeyer/sqlite3-mapnik), or a developer has written a new plugin for mapnik that TileMill does not know about and wants to read that data into TileMill.

It is this latter case where I noticed this problem, as I've written a new csv plugin (http://trac.mapnik.org/ticket/902) and was able to test it in TileMill with no code changes to TileMill simply by passing type=csv. However, the option is not preserved such that it must be set each time the layer is saved.

@ghost ghost assigned springmeyer and willwhite Oct 11, 2011
@springmeyer
Copy link
Member Author

this should be able to be fixed with:

diff --git a/models/Layer.bones b/models/Layer.bones
index 26f5fcc..731d71b 100644
--- a/models/Layer.bones
+++ b/models/Layer.bones
@@ -99,9 +99,8 @@ model.prototype.validateAsync = function(attributes, options) {

 model.prototype.advancedDatasourceOptions = function() {
     var omit = [
-        'type', 'file',
-        'table', 'host', 'port', 'user', 'password', 'dbname',
-        'extent', 'key_field', 'geometry_field', 'type', 'attachdb',
+        'file', 'table', 'host', 'port', 'user', 'password', 'dbname',
+        'extent', 'key_field', 'geometry_field', 'attachdb',
         'srs', 'id', 'project'
     ];
     var advancedOptions = [];
@willwhite
Copy link
Contributor

@springmeyer, is this field autodetected for all files? Since we're not stripping it anymore, I'm seeing it appear in the "advanced" field for shapefiles and sqlite files, but not for KML, GeoJSON, or CSV. If we're going to show the autodetected value in the advanced it would be nice to be consistent with it across all datasource types.

@willwhite willwhite reopened this Oct 19, 2011
@springmeyer
Copy link
Member Author

I've not seen it display/appear in the "advanced" field unless I've added it - if you are seeing that then it is unintended and we can revert this. My only intention was to allow it to be persistent across saves if explicitly added by the user in the UI.

type should be autodetected by millstone for all layer types otherwise the XML passed to mapnik will be invalid (since mapnik Datasources need a type to tell mapnik which dynamic plugin to dispatch to.).

@willwhite
Copy link
Contributor

Ok I think, we can revert this and come back to it. The approach could be to filter it out unless we don't explicitly recognize the type as one of the formats we can autodetect.

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