-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
Allow architecture diagrams to support any character for titles #5929
base: develop
Are you sure you want to change the base?
Allow architecture diagrams to support any character for titles #5929
Conversation
|
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
commit: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5929 +/- ##
==========================================
- Coverage 4.49% 4.49% -0.01%
==========================================
Files 382 383 +1
Lines 53914 53952 +38
Branches 621 596 -25
==========================================
Hits 2425 2425
- Misses 51489 51527 +38
Flags with carried forward coverage won't be shown. Click here to find out more.
|
66f4f73
to
7857686
Compare
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.
Allowing all characters seems okay at the first glance. Love the unit tests!
We should unify the string handling approach, and support markdown as well. (We already support it in flowcharts and some others, so the same functions should be used).
We should also make the grammar stricter to disallow architecture-beta
and group
in the same line. As the grammar is in beta, we can make breaking changes.
After this is finalized, we also need E2E tests and documentation updates, which show the capabilities in examples.
The latest updates on your projects. Learn more about Argos notifications ↗︎ Waiting for the first build to start… |
Good catch on the ambiguous grammar! If markdown support is planned to be added we may also want to consider KaTeX support. Other then the above request changes everything looks good to me. |
Added markdown support. It already had the ability to render, so added grammar rules for |
3b2066b
to
ee87aca
Compare
Architecture diagrams with Markdown
ee87aca
to
67d928f
Compare
📑 Summary
Allows any character between
[
and]
for architecture diagram titles.Resolves #5928
📏 Design Decisions
Change of regex to allow any character. Also had to tweak the
entry Architecture
rule as tests were claiming ambiguity.📋 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:
.