feat(gantt): support multiple excludes and includes lines#7772
feat(gantt): support multiple excludes and includes lines#7772devareddy05 wants to merge 2 commits into
Conversation
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 2d5c428 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 |
@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 #7772 +/- ##
==========================================
+ Coverage 3.26% 3.28% +0.01%
==========================================
Files 599 600 +1
Lines 60839 60857 +18
Branches 917 921 +4
==========================================
+ Hits 1986 1997 +11
- Misses 58853 58860 +7
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 ↗︎
|
|
Hi @devareddy05, thanks for this PR that improves the Gant-diagrams. Some things to adress: [sisyphos-bot] What's working well 🎉 mergeTokens is clean and well-designed. The lowercase normalization, whitespace/comma splitting, and Set-based deduplication are all correct. Switching from replace-semantics 🎉 The infinite loop guard in fixTaskDates is a good defensive addition. The 10,000-day cap and the informative error message (Failed to find a valid date…) turn an infinite 🎉 Changeset and docs are present. minor bump with feat: prefix is correct for new syntax. Things to address 🟡 [important] vert task order change is undocumented and untested ganttDb.js lines 578–583 contain a behavior change that isn't mentioned in the PR description: // Before (inferred) // After The fix is correct — vert tasks consumed lastOrder slots, creating invisible row gaps in non-vert task layout (because ganttRenderer.js:246 filters vert tasks out with Suggested addition — unit test in ganttDb.spec.ts: it('should not consume order slots for vert tasks', () => { 🟡 [important] No Cypress E2E test for multi-line excludes The core feature — writing excludes or includes on multiple lines — has unit tests for DB logic but no visual regression test. Since this changes rendered gantt chart output Suggested test in cypress/integration/rendering/gantt.spec.ts: Security No XSS or injection issues introduced by this PR. The mergeTokens tokens are consumed exclusively as comparison strings in isInvalidDate — they never reach any DOM sink. Error (One pre-existing concern noted but out of scope: todayMarker.replace(/,/g, ';') at renderer line ~867 sets a style attribute from user-supplied text without sanitization — |
|
Thanks for the review @pbrolin47. On the two points raised by sisyphos-bot: vert task order change — this is not part of this PR. The diff ( No Cypress E2E test for multi-line excludes — fair point, added in Let me know if you would like anything else changed. |
📑 Summary
Allow Gantt diagrams to split long
excludes(andincludes) lists across multiple lines, optionally grouped with%%comments:gantt dateFormat DD-MM-YYYY excludes weekends %% week 7 is winter break excludes 10-02-2025 11-02-2025 12-02-2025 %% workers holiday 1 maj excludes 01-05-2025Previously each
excludesline replaced the previous one.Resolves #6270
Changes
ganttDb.js:setExcludes/setIncludesnow merge tokens via a shared helper withSet-based dedupe. Single-line usage is unchanged; repeated lines concatenate.ganttDb.spec.ts: refreshed the "should not infinite loop when excluding everything" test that previously relied on replace-semantics; it now usesclear()between scenarios.ganttDb.spec.tsandparser/gantt.spec.jscovering merge, dedupe, includes, and the issue diagram.docs/syntax/gantt.md: documented the multi-line form.Testing
pnpm exec vitest run packages/mermaid/src/diagrams/gantt/-> 76 pass, 1 skipped (timezone-gated).📋 Tasks