magritte-metamodel / magritte

The Magritte Meta-Model
65 stars 34 forks source link

MAContainer not working with pragmas - regression by changes from GToolkit #337

Closed JanBliznicenko closed 3 months ago

JanBliznicenko commented 1 year ago

Since https://github.com/magritte-metamodel/magritte/commit/9842df75c0f5e728d76892fa0f3b55bdd1f6b840, descriptions using MA(Priority)Container no longer work, because not all description classes implement definingContext: sent by MAPragmaBuilder>>#setDefiningContextOf:to:in: sent by MAPragmaBuilder>>#buildDescriptions:

I believe definingContext: should be implemented by MADescription (or defined as subclass responsibility and implemented in MAContainer as well).

obrazek

seandenigris commented 1 year ago

Thanks for the report. That's probably my fault. I vaguely remember that it was originally implemented in the superclass. So I fully understand, what is the scenario that causes the error? The tests are all passing, so we should also add a test for that use case.

JanBliznicenko commented 1 year ago
| description |

Object subclass: #MAMockWithContainer
    instanceVariableNames: ''
    classVariableNames: ''
    package: 'Magritte-Tests-Model-Mocks'.

(Smalltalk at: #MAMockWithContainer) compile: 'descriptionWithContainer
    <magritteDescription>
    ^ MAContainer new'.

description := (Smalltalk at: #MAMockWithContainer) new magritteDescription.

self assert: description label isNotEmpty

However, when I look at other usages of MAContainer, they seem to be always used as <magritteContainer>, not as <magritteDescription>, even if they are descriptions (subclass of MADescription), so I wonder if I just do not misuse the container

seandenigris commented 12 months ago

I was thinking that I hadn't seen containers used that way. Do you have a use case that is not supported by the existing behavior?

stale[bot] commented 7 months ago

To limit bug bankruptcy (see https://www.joelonsoftware.com/2012/07/09/software-inventory/) this issue has been automatically marked as stale because it has not had any activity in 6 months. If no further activity occurs, it might be closed. If this issue remains important to you, please comment to reactivate the issue. Thank you for your contributions.

JanBliznicenko commented 3 months ago

You are right that I might not be using the containers correctly and this was more of a coincidence that it used to work.