Conversation
…shapes-compatible-with-drawio-dark-mode
| def updateShapeData(shape, master): | ||
| updated = False | ||
| if None == master: | ||
| return |
There was a problem hiding this comment.
If None==master is a fatal mistake, should you report it for debug purposes? Otherwise it will fail silently later (because the None evaluates the same as False)
There was a problem hiding this comment.
It's not a fatal mistake - it should just means that, for that shape, the export script couldn't find its counterpart in the shape library, so the shape won't be updated to match its library counterpart. That update would purely aesthetic, so functionality isn't impacted. This could happen if a user is using a shape that's not in the GUNNS repo shape library (shape libraries outside of GUNNS do exist and are used), or if a user has a custom shape. I do think it would be more clear if it returned the updated flag though, which would be False at that point.
I was using updateLinkShapeData() and updateSpotterShapeData() as templates. However, do I think it could make sense to log/report when the master shape can't be found.
| # checks, should be a little faster... | ||
| $(GUNNSDRAW_HEADERS) : %.hh : %.xml | ||
| @ echo $(shell $(PYTHON) $(GUNNS_HOME)/draw/netexport.py $<) | ||
| @ echo '$(shell $(PYTHON) $(GUNNS_HOME)/draw/netexport.py $<)' |
There was a problem hiding this comment.
don't the quotes just make it print $(GUNNS_HOME) instead of /Users/jvalerio/Sandbox/gunns_workspace/gunns?
There was a problem hiding this comment.
Hmm, I made that change so the CI would print properly - now that I'm printing out shape data updates, they often contain parenthesis (see here) which confuses the echo statement resulting in an error that isn't the export script's fault (see here). So, it works properly in the CI but I will try some local testing.
…shapes-compatible-with-drawio-dark-mode
|
I decided to regenerate all the repo-tracked drawing XMLs using the new |
|
I would like to note that Adsorber and MetabolicsV4 links used in TestOverflow40, TestOverflow41, and TestOverflow43 are currently housed in an external library in a different repository. I have updated that library for dark mode and will push it to that repo in the future. |
Ok cool, thanks! |
…ed by export script
…shapes-compatible-with-drawio-dark-mode
…gunnsdraw-shapes-compatible-with-drawio-dark-mode
|
I made an update to the GunnsFluidAccum shape on master that I knew would cause merge conflicts with your branch. I think I have addressed them all and pushed an update to 166-make-gunnsdraw-shapes-compatible-with-drawio-dark-mode. |
Just needed to revert the About fields, otherwise looks good 👍 |
2 participants
Updated GunnsDraw libraries shapes to be compatible with both draw.io dark mode and light mode. Updated netexport.py and supexport.py to update old gunns drawings shapes to the new shapes in the libraries.
closes #166
closes #154
closes #149
TO DO