fix(er): recognize '1' cardinality alias before relationship operators#7375
Conversation
The '1' alias for 'only one' cardinality was not recognized when directly followed by relationship operators like '--' or '..'. This occurred because the lexer rule only matched '1' when followed by whitespace and then a letter. This fix adds a new lexer rule to also recognize '1' when directly followed by the relationship operators (--, .., .-, -.). Fixes mermaid-js#7351
🦋 Changeset detectedLatest commit: d347f77 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7375 +/- ##
==========================================
- Coverage 3.58% 3.58% -0.01%
==========================================
Files 474 475 +1
Lines 47529 47540 +11
Branches 740 740
==========================================
Hits 1706 1706
- Misses 45823 45834 +11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
omkarht
left a comment
There was a problem hiding this comment.
Suggestion: Consider adding visual rendering tests in cypress/integration/rendering/erDiagram.spec.js to verify the fix renders correctly. This would help catch any visual regressions.
Also, since the regex covers all 4 relationship operators (--, .., .-, -.), it would be good to add unit tests for 1.- and 1-. as well for completeness.
|
Addressed the review feedback:
Thanks for the thorough review! |
There was a problem hiding this comment.
Thanks for this contribution, nice to see the tests covering all operators too. I see that a changeset has been included, From my side this looks good, thanks again for the fix.
Please refer #7375 (review)
omkarht
left a comment
There was a problem hiding this comment.
Kindly resolve the failing test case
…DENTIFYING) Per the jison grammar, -. (solid-to-dotted) returns NON_IDENTIFYING. The test comment and expectation were incorrect.
|
Fixed the failing test - the expectation was incorrect. Per the jison grammar, |
omkarht
left a comment
There was a problem hiding this comment.
Thanks for the clarification and for aligning the test with the jison grammar.
The update looks good to me now, all tests are passing
|
@kaigritun, Thank you for the contribution! |
|
I'm confused about this change. I can't reproduce the error described in the original issue in v11.12.2 or v11.12.3, but I am noticing that in v11.13.0, |
Summary
Fixes #7351
The
1alias foronly onecardinality was not being recognized when directly followed by relationship operators like--or...Example that failed before this fix:
Root cause
The lexer rule for the
1alias only matched when1was followed by whitespace and then a letter:This meant
1 toworked (space + letter) but1--didn't (no space, operator directly follows).Fix
Added a new lexer rule to also recognize
1when directly followed by the relationship operators (--,..,.-,-.):Tests
Added two test cases covering:
1--(identifying relationship)1..(non-identifying relationship)Checklist