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 Semicolons in Timeline Titles, Sections, Periods, and Events #5335
base: develop
Are you sure you want to change the base?
Allow Semicolons in Timeline Titles, Sections, Periods, and Events #5335
Conversation
1. Removed the semicolon from the regex for titles, sections, periods, and events. Changes to timeline.spec.js 1. Added a test for the changes made in the timeline parser
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #5335 +/- ##
=======================================
Coverage 5.74% 5.74%
=======================================
Files 277 276 -1
Lines 41899 41888 -11
Branches 489 514 +25
=======================================
Hits 2407 2407
+ Misses 39492 39481 -11
Flags with carried forward coverage won't be shown. Click here to find out more. |
timelineDB.getTasks().forEach((t) => { | ||
switch (t.task.trim()) { | ||
case ';ta;sk1;': | ||
expect(t.events).to.deep.equal([';ev;ent1;']); | ||
break; | ||
|
||
case ';tas;k2;': | ||
expect(t.events).to.deep.equal([';eve;nt2;', ';event;3;', ';eve;nt4', ';even;t5;']); | ||
break; | ||
|
||
default: | ||
break; | ||
} | ||
}); | ||
}); |
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.
instead of a loop with switch case, it'll be cleaner to access the tasks by index.
1. Re-organized the new TL-6 test to not use a for-loop with a switch case. 2. Imported the commonDb module to ensure titles are be tested. 3. Use indices to retrieve elements that need to be tested.
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 changed the test so that it uses indices to access events instead of using a for-loop with a switch case which was unnecessary.
📑 Summary
This pull request removes the semicolons from the title, section, events, and periods regex so that semicolons can be used in their text content.
Resolves #4175. This doesn't directly address that issue, but I wanted to link it because my ultimate goal is to figure out how to allow colons without them being identified as tokens.
📏 Design Decisions
I simply removed the semicolon symbol from the exclusion clause regex for the title, section, event and period. I almost made sure to add a test to the timeline test suite and run
pnpm test
to ensure that it was still working.📋 Tasks
Make sure you
MERMAID_RELEASE_VERSION
is used for all new features.develop
branch