Fix duplicate names in composite components #84
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a consequence of #63, a composite component's subcomponents would not get unique names for their geometry coordinate systems if rendered directly to a Cell (i.e., if
render!
were called withoutbuild!
, which replacescomp
withgeometry(comp)
in references). Even prior to that,Path
components would get turned into coordinate systems with the path's name, not the unique geometry name, and this would happen whetherbuild!
was called first or not.This fix does a couple things to ensure different components get rendered as coordinate systems with unique names (whether
build!
is called or not). First, when we encounter a component while rendering to aCell
, we use itscoordsys_name
(unique by construction, usuallyname(geometry(comp))
) rather than its (non-unique)name(comp)
to name theCell
that holds its rendered elements and references.This doesn't quite solve the
Path
problem because of the slightly hacky_geometry!(cs, path)
, which adds the path itself to an intermediate coordinate systempath._geometry
with a unique name. (We do this for some complicated and subtle reasons where I couldn't find a better solution.) So if we definedcoordsys_name(path) = name(geometry(path))
, then we'd still end up with a stack of two identically namedCell
s -- the wrapper frompath._geometry
, and (in its references) the one containing the path's rendered elements and references. To get around this,coordsys_name(::Path)
checks whetherpath._geometry
contains thePath
already. If it does, then we useuniquename(name(path))
for the second cell. This is also a little hacky, but as long as we understand that this new functioncoordsys_name
is meant only for this rendering context I don't see it causing any problems.