tangrams / tangram-play

Text editor web app for Tangram scenes
https://tangram.city/play/
MIT License
96 stars 27 forks source link

YAML references clobbered #118

Closed matteblair closed 9 years ago

matteblair commented 9 years ago

YAML allows repeated values by specifying "anchors" with "&" and "references" with "*". This can be very useful in stylesheets where you want several colors to match without manually updating all of them when you want to make a change.

screen shot 2015-07-27 at 2 38 16 pm

Tangram-Play correctly supports referenced values already! However if the color picker is used, the anchor for color is clobbered and all the references become broken.

screen shot 2015-07-27 at 2 42 17 pm

Since the syntax for specifying anchors is very simple (just &\w+) we should check for them before totally replacing the value in the color node (or any node, really). A good parser will even tell you if a node is an anchor or a reference.

louh commented 9 years ago

I had no idea YAML could do this. :open_mouth:

So on references, should a color picker also be displayed? The potentially annoying part of the implementation detail is that the color picker (and all widgets in the editor, really) are only aware of its own local key-value pair (and some other contextual information regarding the location of that pair) so there needs to be an intermediate step where if it looks like the value is a reference, it must request the actual color value from somewhere else. And if the color picker changes the value, does it remove the reference (thus preserving its original value on the anchor) or does it also update the anchor with it?

matteblair commented 9 years ago

I actually think it's not a bad thing that there's no color picker on the references - I think of them as "borrowing" the original value so it makes sense to me that only the original is mutable.

Handling anchors and references gracefully will depend somewhat on how much meta-information the yaml parser provides, and this is the part I don't know. If the parser can tell you that a specific node is an anchor or a reference then it should be simple to check for anchors before setting a value and then maintaining the anchor (our c++ yaml parser provides this sort of information, that's what I'm mentally modelling this on). However if the parser spits a purely associative-array interpretation of the yaml document then we might be out of luck - the anchor values may not even be accessible in that case.

patriciogonzalezvivo commented 9 years ago

This is incredible feature! I also had no idea. Tangram.js has the proper YAML parser that is detecting the color references. The YAML "parser" we have in tangram-play side is a bunch of RegEx functions I put together that stores the relationship between keys and lines, (something that the proper parser doesn't do). To give more suport to this kind of features we should use more the already parsed data from Tangram.js

mjcunningham commented 9 years ago

This is great! Dealing with the UX will be a little tricky. What if someone declares the color in the middle of the file and references it above and below? How do we deal with letting someone break a reference on purpose but not break it by accident? Let's talk about it in the meeting tomorrow.

matteblair commented 9 years ago

I should mention that this anchor-reference mechanic works for any YAML node. We only need to pay special attention in the places where we programmatically set the value of nodes. If the editor ui is just applying a regex to the string value of a node, it should be very simple to just leave anchor values untouched by programmatic changes.

patriciogonzalezvivo commented 9 years ago

Should I close this issue? Moving towards multi yaml files with an include system, will make references even harder to support on TP.

matteblair commented 9 years ago

Based on our experiences with eraser map it seems highly impractical to author a thorough stylesheet without some mechanism that does the job of anchors and references, so I think it would be counter-productive to create a bad user experience for someone using the anchor-reference syntax.

Is this a very difficult thing to fix? It seems to me that all Tangram Play needs to do is to not modify any text that fits the pattern of a YAML anchor when a widget is changing a value. Something like ^\s*(&\w+)\s+.*$ should match an anchor in a node.

patriciogonzalezvivo commented 9 years ago

Ok, I will take a look to it

patriciogonzalezvivo commented 9 years ago

Resolved on https://github.com/tangrams/tangram-play/pull/229 00

matteblair commented 9 years ago

Superb, thank you!

patriciogonzalezvivo commented 9 years ago

my pleasure : )