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

Ensure enumeration values are rendered correctly #3

Merged
merged 1 commit into from
Feb 25, 2024

Conversation

szschaler
Copy link
Contributor

This works around a bug in EMF, where enumeration attributes that hold the default value are reported as unset (even though by default enumeration attributes are set to not be unsettable).

@kolovos
Copy link
Member

kolovos commented Feb 20, 2024

This is also the case for non-enumeration attributes. For example, in https://eclipse.dev/epsilon/playground/?9e36d1d1 the name attribute of the first box is marked as unset as the provided and the default values ("B1") match. If we want to make some/all attributes with default values appear on the diagram, I think we should do this using @diagram annotations as we already do for other customisations e.g.

@namespace(uri="bl")
@diagram(showDefaultValues="true") // Enable for all attributes
package bl;

class Box {
	attr String name = "B1";
	@diagram(showDefaultValue="true") // Enable for this specific attribute
	attr Color color;
}

@szschaler
Copy link
Contributor Author

Huh. It gets even weirder: in that example, if I change the EOL program to

var box = Box.all.first();

box.isSet("name").println();
box.isUnsettable("name").println();

box.isSet("color").println();
box.isUnsettable("color").println();
box.color.println();

operation Any isSet(attr : String) {
    var eAttribute = self.eClass().getEStructuralFeature(attr);
    return self.eIsSet(eAttribute);
}

operation Any isUnsettable(attr : String) {
    var eAttribute = self.eClass().getEStructuralFeature(attr);
    return eAttribute.isUnsettable();
}

then I get

false
false
false
false
Red

In other words, EMF has correctly marked each of these attributes as not unsettable, and still maintains that they haven't been set -- which feels like a contract violation. My proposed fix to the renderer would catch this case, too, though, in a way that I feel would make the visualisation more intuitive.

Given that this is just in the context of the education platform / the playground, I don't think there's a need for special markup in the meta-model, but that's of course a decision for you to make.

The change I'm proposing here is the same I have implemented in the Xtext tool's visualisation of Xtext models.

@kolovos
Copy link
Member

kolovos commented Feb 20, 2024

Doesn't making this change defeat the purpose of checking for e.eIsSet(attr)? For example, if I change the model as follows, the default value of the (unset) name attribute of the first box appears on the model diagram.

<?nsuri bl?>
<_>
    <box color="red"/>
    <box name="B2" color="green"/>
</_>

@szschaler
Copy link
Contributor Author

But isn't that what one would expect to see? After all, that's the value of the object's attribute at that point, regardless of how it got set. However, if the attribute is unsettable and eIsSet() returns false, then the value is null or undefined. In that case, it feels OK to leave out the attribute altogether. Alternatively, one may wish to generate a line stating that something like attr = undefined.

@agarciadom
Copy link
Member

Looks good to me, thank you!

I'll rebase this on top of the latest tip and then merge it.

@agarciadom agarciadom merged commit f715eb5 into epsilonlabs:main Feb 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants