Skip to content

[1819] Allow multiplicityExpression after redefinitionExpression subs… #1829

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.

Sign up for GitHub

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

Jump to bottom
Open
mcharfadi wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[1819] Allow multiplicityExpression after redefinitionExpression subs… #1829

mcharfadi wants to merge 1 commit into from
+717 −358

Conversation

@mcharfadi
Copy link
Contributor

@mcharfadi mcharfadi commented Jan 12, 2026 •
edited
Loading

…ettingExpression and typingExpression

Bug: #1819

PLEASE READ ALL ITEMS AND CHECK ONLY RELEVANT CHECKBOXES BELOW

Auto review

  • Have you reviewed this PR? Please do a first quick review, It is very useful to detect typos and missing copyrights, check comments, check your code... The reviewer will thank you for that :)

Project management

  • Has the pull request been added to the relevant milestone?
  • Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
  • Have the relevant issues been added to the pull request?
  • Have the relevant labels been added to the issues? (area:, type:)
  • Have the relevant issues been added to the same project milestone as the pull request?

Changelog and release notes

  • Has the CHANGELOG.adoc + doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc been updated to reference the relevant issues?
  • Have the relevant API breaks been described in the CHANGELOG.adoc?
  • Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
  • In case of a change with a visual impact, are there any screenshots in the doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?
  • In case of a key change, has the change been added to Key highlights section in doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?

Documentation

  • Have you included an update of the documentation in your pull request? Please ask yourself if an update (installation manual, user manual, developer manual...) is needed and add one accordingly.

Tests

  • Is the code properly tested? Any pull request (fix, enhancement or new feature) should come with a test (or several). It could be unit tests, integration tests or cypress tests depending on the context. Only doc and releng pull request do not need for tests.

@mcharfadi mcharfadi added this to the 2026.1.0 milestone Jan 12, 2026
@mcharfadi mcharfadi requested a review from AxelRICHARD January 12, 2026 10:56
@mcharfadi mcharfadi linked an issue Jan 12, 2026 that may be closed by this pull request
Allow to edit multiplicity range before or after the Subsetting/FeatureTyping... #1819
Open
@mcharfadi mcharfadi force-pushed the mch/enh/direct-edit-ultiplicity branch 4 times, most recently from 1a1edf8 to 35c54a1 Compare January 12, 2026 12:12
@mcharfadi mcharfadi marked this pull request as ready for review January 12, 2026 13:07
AxelRICHARD
AxelRICHARD reviewed Jan 12, 2026
View reviewed changes
.../java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDirectEditTests.java
.verify(Duration.ofSeconds(10));
}

@DisplayName("GIVEN a diagram with a part, WHEN we direct edit with a part with multiplicity operation, THEN the part is correctly set")
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We agreed that this was not part of the issue.
Add a multiplicity range at the end after a value with a unit is not allowed by the SysMLv2 specification, so I don't think it is a good idea to authorize such edit.

Copy link
Contributor Author

@mcharfadi mcharfadi Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually tested that the grammar was not allow allowing to set a multiplicity after an operation see line 618, it will send an error message to the frontend.

Copy link
Contributor Author

@mcharfadi mcharfadi Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not authorizing it, actually I added the DiagramDirectEditErrorListener to capture the fact that it was not allowed.
See line 618 where I test that using a multiplicity range after a value unit will send an error message since the parser is not allowing this case.

Copy link
Member

@AxelRICHARD AxelRICHARD Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I see it now, thank you.

Copy link
Member

@AxelRICHARD AxelRICHARD Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So could you please edit the @DisplayName of this test, because here you test that it is not possible to edit the multiplicity after the feature value, and that is not what is it written in the @DisplayName

@mcharfadi mcharfadi force-pushed the mch/enh/direct-edit-ultiplicity branch 4 times, most recently from 53edfa7 to d7c872f Compare January 13, 2026 15:58
@mcharfadi
[1819] Allow multiplicityExpression after redefinitionExpression subs…
ace9b62 
…ettingExpression and typingExpression

Bug: #1819
Signed-off-by: Michaël Charfadi <michael.charfadi@obeosoft.com>
@mcharfadi mcharfadi force-pushed the mch/enh/direct-edit-ultiplicity branch from d7c872f to ace9b62 Compare January 13, 2026 16:11
AxelRICHARD
AxelRICHARD reviewed Jan 14, 2026
View reviewed changes
doc/content/modules/user-manual/pages/release-notes/2026.1.0.adoc
When a `PartUsage` is selected, the new actor created (i.e. a `PartUsage`) will subset the selected `PartUsage`.
- In diagrams, when using _direct edit_ tool, you can now set a _multiplicity range part_ [n..n] after or before a _typing part_, _subsetting part_ or _redefinition part_.
Before, you could only do `part p2[4] :> p1`, `part p6[1] :>> p1` or `part p8[1] : DefA`
Now you can also do : `part p3 :> p1[4]`, `part p7 :>> p1[1]` or `part p9 : DefA[1]`
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please use the same examples as on the "Before" line, it will help to understand.
  • no space before a ':'

.../java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDirectEditTests.java
.verify(Duration.ofSeconds(10));
}

@DisplayName("GIVEN a diagram with a part, WHEN we direct edit with a part with multiplicity operation, THEN the part is correctly set")
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I see it now, thank you.

.../java/org/eclipse/syson/application/controllers/diagrams/general/view/GVDirectEditTests.java
.verify(Duration.ofSeconds(10));
}

@DisplayName("GIVEN a diagram with a part, WHEN we direct edit with a part with multiplicity operation, THEN the part is correctly set")
Copy link
Member

@AxelRICHARD AxelRICHARD Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So could you please edit the @DisplayName of this test, because here you test that it is not possible to edit the multiplicity after the feature value, and that is not what is it written in the @DisplayName

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AxelRICHARD AxelRICHARD AxelRICHARD left review comments

Assignees

No one assigned

Labels

pr: to review later priority: low

Projects

None yet

Milestone

2026.1.0

Development

Successfully merging this pull request may close these issues.

Allow to edit multiplicity range before or after the Subsetting/FeatureTyping...

3 participants

@mcharfadi @AxelRICHARD