Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

review deep copy change to feature_type_style required by carto parser #946

Closed
springmeyer opened this Issue · 6 comments

3 participants

@springmeyer
Owner

A change was needed by Colin to allow a deep copy of rules in feature_type_style. Latest carto-parser needs this to compile, so will merge into trunk to enable master to work for carto parser.

But I think we should review this in more detail.

@springmeyer
Owner

now merged. these were the changes that came in (with the last being the one that looks to need review/touchup):

a321ec7
03d809b
48311f3

@springmeyer
Owner

@rundel - possible for you to comment a bit on the background of this?

@rundel
Collaborator

These changes came up when I was working on style / symbolizer inheritance (rundel/carto-parser#19) with the carto parser since some attributes are stored in shared pointers it was necessary to explicitly create a new copy of these attributes as otherwise updating the child style changes the attribute in the parent. Implementation was quick and dirty but I tried to do it in a way that shouldn't affect anything else. I think I got all the relevant shared pointers but I may have missed something.

The first two patches are just some missing attribute setters.

@artemp artemp was assigned
@springmeyer
Owner

assigning to artem so he can take a look. My hunch is we'll want to revise this, or remove it, when we actually merge the carto parser code into master.

@springmeyer
Owner

/cc @artemp as I know he was looking at cleaning this up recently.

@kkaefer kkaefer referenced this issue from a commit in kkaefer/mapnik
@springmeyer springmeyer keep proper type in text sym deep copy - refs #946 0729110
@springmeyer
Owner

closing, planning on removing what remains of these changes (they are broken anyway by changes in text stuff) and will track a proper solution at #2081

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.