-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix: Detector order #4124
fix: Detector order #4124
Conversation
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.
I'm not super experienced with the whole registerLazyLoadedDiagrams
code, but so far, it looks good to me!
By the way, is it also worth making the detectors for flowchart
and stateDiagram
stricter (maybe all other diagrams too)?
E.g. checking for /flowchart\s/
(must end with a white-space character)
I'm guessing that:
```mermaid
stateDiagram-blah-blah-blah
[*] --> Still
Still --> [*]
Still --> Moving
Moving --> Still
Moving --> Crash
Crash --> [*]
```
should probably throw a unrecognized diagram error, instead of the current error.
After all, somebody might make a custom flowchart-v100
diagram type as a plugin, and I believe that then their external usage of registerLazyLoadedDiagrams()
would be called after we've already called registerLazyLoadedDiagrams()
internally.
* develop: Update docs fix Lint Update CHANGELOG.md Update CHANGELOG.md fix: fix exports Doc (typo): remove duplicate "be" Fix readme link Regenerate mermaid docs Add deepdwn to cspell Add Deepdwn to native integrations list
Co-authored-by: Alois Klink <[email protected]>
Making it stricter now might break many existing diagrams. So I'd prefer we don't mess with that. |
It won't for the It's probably something that's low priority, so it shouldn't go in this PR. |
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.
Look great, thanks @sidharthv96 !
📑 Summary
Rearrange detectors so that diagrams are properly detected.
Resolves flowchart-elk not being detected
📏 Design Decisions
Diagrams with more specific detectors should come on top of the list.
eg, when matching
flowchart-elk
from diagram text, ifflowchart
detector is checked beforeflowchart-elk
's detector, the diagram would incorrectly be detected as flowchart.So, we need to check
flowchart-elk
first. Same case with-v2
diagrams.📋 Tasks
Make sure you
develop
branch