fix(eventmodeling): preserve last character of data spec content#7936
Open
IvanTheGeek wants to merge 1 commit into
Open
fix(eventmodeling): preserve last character of data spec content#7936IvanTheGeek wants to merge 1 commit into
IvanTheGeek wants to merge 1 commit into
Conversation
Both data-spec strip sites in eventmodeling/db.ts used
`substring(0, lastIndexOf('}') - 1)`. Since `String#substring`'s end
index is already exclusive, `lastIndexOf('}')` alone excludes the
brace — the extra `- 1` chopped one more character. For inline specs
that was the last content character ({item, quantity} rendered as
"item, quantit"); for block references it silently ate the newline
before the closing brace.
Extract the two operations as exported stripInlineValue /
stripBlockValue helpers and unit-test them, mirroring the fix merged
in the source project (lgazo/event-modeling-tools#20) so both
codebases stay in sync.
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 178922f 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 |
github-actions
Bot
added
the
Type: Bug / Error
4 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefactors data-spec framing removal in eventmodeling diagrams by extracting ChangesEventmodeling data spec truncation fix
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@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 #7936 +/- ##
===========================================
- Coverage 77.42% 74.99% -2.43%
===========================================
Files 564 542 -22
Lines 74900 66792 -8108
Branches 12659 12448 -211
===========================================
- Hits 57988 50092 -7896
- Misses 15915 16688 +773
+ Partials 997 12 -985
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 ↗︎
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📑 Summary
Data-spec labels in
eventmodelingdiagrams drop their last character:{item, quantity}renders asitem, quantit.Both strip sites in
packages/mermaid/src/diagrams/eventmodeling/db.tsusedsubstring(0, lastIndexOf('}') - 1).String#substring's end index is already exclusive, solastIndexOf('}')alone excludes the brace — the extra- 1chopped one more character. For inline specs that's the last content character (user-visible truncation); for block references it silently ate the newline before the closing}(and would eat a content character for any value not ending in a newline).Resolves #7935
📏 Design Decisions
stripInlineValue/stripBlockValue), same semantics — both codebases stay in sync.calculateTextPropsinto exported helpers so they can be unit-tested directly; call sites otherwise unchanged (sanitize/wrap/escape pipeline untouched).}(upstream-blessed semantics; the old- 1happened to eat it). DownstreamwrapLabel→ HTML is insensitive to it.Tests
Ported the 7 unit tests from event-modeling-tools#20 into
eventmodeling.spec.ts(inline + block notations, single-char body, empty body, whitespace preservation).- 1reintroduced: 6/7 new tests fail, proving regression coverage (the one that passes either way is the empty-body{}case — nothing to chop).📋 Tasks
Make sure you
MERMAID_RELEASE_VERSIONis used for all new features. — N/A, bug fix, no new syntax or feature.mermaid: patch,fix:prefix).Fixed the eventmodeling data-spec truncation bug so labels like
{item, quantity}now keep their final character.What changed:
stripInlineValuestripBlockValueThis addresses the off-by-one caused by
substring(0, lastIndexOf('}') - 1).