-
-
Notifications
You must be signed in to change notification settings - Fork 8k
make flowchart elk detector regex match less greedy #6796
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
make flowchart elk detector regex match less greedy #6796
Conversation
🦋 Changeset detectedLatest commit: fb890a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6796 +/- ##
=======================================
Coverage 3.71% 3.71%
=======================================
Files 454 454
Lines 44733 44733
Branches 708 708
=======================================
Hits 1660 1660
Misses 43073 43073
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 ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a regex bug in the flowchart elk detector that was causing incorrect diagram type detection when the default renderer is set to 'elk'. The greedy regex pattern was matching any text containing 'graph' anywhere, interfering with other diagram types that contain this character sequence.
- Fixed regex pattern to use proper grouping instead of character class matching
- Added comprehensive unit and integration tests to prevent regression
- Ensures diagram types like mindmap, classDiagram, and erDiagram are correctly detected even when containing 'graph' in their content
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/mermaid/src/diagrams/flowchart/elk/detector.ts | Fixed regex pattern from /^\s*flowchart|graph/ to /^\s*(flowchart|graph)/ for proper grouping |
packages/mermaid/src/diagram-api/diagram-orchestration.spec.ts | Added unit tests verifying correct diagram detection for mindmap, class, and ER diagrams |
cypress/integration/rendering/mindmap.spec.ts | Added integration test for mindmap with 'Photograph' label containing 'graph' |
cypress/integration/rendering/erDiagram.spec.js | Added integration test for ER diagram with 'Photograph' entity containing 'graph' |
cypress/integration/rendering/classDiagram.spec.js | Added integration test for class diagram with 'demographicProfile' property containing 'graph' |
@HashanCP, Thank you for the contribution! |
📑 Summary
Makes the flowchart elk detector less greedy. Having it without group match will detect diagram type incorrectly and will not allow to have char sequence 'graph' in any diagram type as described in #6795
Resolves #6795
📋 Tasks
Make sure you
MERMAID_RELEASE_VERSION
is used for all new features.pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.