Skip to content
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

Changes to Gantt Parsers to allow hashes and semicolons to titles, sections, and task data. #5095

Merged
merged 8 commits into from
Jan 26, 2024

Conversation

FutzMonitor
Copy link
Contributor

@FutzMonitor FutzMonitor commented Dec 2, 2023

📑 Summary

This PR makes changes to the gantt.jison file that allows users to use hash and semicolons in the title, section, and task title.

Resolves #1981

📏 Design Decisions

Changes to gnatt.jison

  1. Removed the hash and semicolon symbols from the title regex to allow for their use.
  2. Removed the hash and semicolon symbols from the section regex to allow fortheir use.
  3. Removed the hash and semicolon symbols for the taskTxt regex to allow for their use. I did not remove the colon because the parser fails to recognize when the actual taskData begins if that distinctions isn't kept.
  4. Removed the regex #[^\n]* which skipped comments to fix some bugs with hash symbols in the taskTxt. I tested this changed by putting it back and using the comment ## hello world to see if it was recognized as a comment, but I would receive a syntax error and the diagram would not be rendered. So, I think we can safely remove that line, BUT it would be best practice if someone else tested this change to ensure that this will not break anyone's Gantt diagrams."

📋 Tasks

Make sure you

1. Removed the hash and semicolon symbols from the title regex to allow for their use.
2. Removed the hash and semicolon symbols from the section regex to allow for their use.
3. Removed the hash and semicolon symbols for the taskTxt regex to allow for their use. I did not remove the colon because the parser fails to recognize when the actual taskData begins if that distinctions isn't kept.
4. Removed the regex \#[^\n]* which skipped comments to fix some bugs with hash symbols in the taskTxt. I tested this changed by putting it back and using the comment  to see if it was recognized as a comment, but I would receive a syntax error and the diagram would not be rendered. So, I think we can safely remove that line, BUT it would be best practice if someone else tested this change to ensure that this will not break anyone's Gantt diagrams.
1. Removed typo
Copy link

netlify bot commented Dec 2, 2023

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 59264a3
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/65b3e286930dd90008642061
😎 Deploy Preview https://deploy-preview-5095--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Dec 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6939cf5) 80.16% compared to head (59264a3) 79.36%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5095      +/-   ##
===========================================
- Coverage    80.16%   79.36%   -0.80%     
===========================================
  Files          167      167              
  Lines        13902    13902              
  Branches       707      707              
===========================================
- Hits         11145    11034     -111     
- Misses        2604     2715     +111     
  Partials       153      153              
Flag Coverage Δ
e2e 85.16% <ø> (-1.01%) ⬇️
unit 43.04% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 12 files with indirect coverage changes

1. Consistent spacing on line 30
@FutzMonitor
Copy link
Contributor Author

Does the Netlify error "Failed during stage Install dependencies": dependency_installation script returned non-zero exit code: 1 have anything to do with the changes that I've made? I'm unsure since it's part of the build.ts and no reference is necessarily made to my changes.

@aloisklink
Copy link
Member

Does the Netlify error "Failed during stage Install dependencies": dependency_installation script returned non-zero exit code: 1 have anything to do with the changes that I've made? I'm unsure since it's part of the build.ts and no reference is necessarily made to my changes.

It looks like Node.JS v18.19.0 broke the ts-node-esm command that we use to build Mermaid, and Netlify just switched to using it by default. So no, it's nothing to do with your PR.

I've made a PR fixing it: #5097.

@FutzMonitor FutzMonitor changed the title Issue1981 Changes to Gantt Parsers to allow hashes and semicolons to titles, sections, and task data. Dec 4, 2023
@FutzMonitor
Copy link
Contributor Author

FutzMonitor commented Dec 5, 2023

When you use # in the live editor, it doesn't break the live editor. However, I believe we have to make this functionality visually consistent to %% by styling the text green.

These changes should be followed with updates to the Gantt documentation because there's no mention of this functionality there [link].

Copy link
Contributor

@nirname nirname left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nirname nirname requested a review from a team January 14, 2024 19:16
Copy link
Member

@sidharthv96 sidharthv96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit tests and rendering tests for the newly supported cases, along with updates to the documentation.

@FutzMonitor
Copy link
Contributor Author

Sounds good. I'll look at the other tests for guidance and update affected documentation.

1. Added a Gantt diagram that demonstrates to users that hashtages and semicolons can be added to titles, sections, and task data.
Changes to gantt.spec.js
1. Added unit tests to ensure that semicolons and hashtags didn't break the functionality of the gantt diagram when used in titles, sections or task data.
Changes to /parser/gantt.spec.js
1. Added rendering tests to ensure that semicolons and hashtags in titles, sections, and task data didn't break the rendering of Gantt diagrams.
Copy link
Contributor Author

@FutzMonitor FutzMonitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some unit tests and rendering tests. I looked through the documentation, and I didn't find any explicit instances mentioning that semicolons and hashtags weren't allowed. So, I didn't add anything in terms of documentation. I did add another graph in gantt.html to demonstrate the semicolons and hashtags are fine.

@sidharthv96 sidharthv96 added this pull request to the merge queue Jan 26, 2024
Merged via the queue into mermaid-js:develop with commit 91907fe Jan 26, 2024
20 checks passed
@FutzMonitor FutzMonitor deleted the issue1981 branch January 26, 2024 18:07
fuxingloh referenced this pull request in fuxingloh/contented Feb 3, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [mermaid](https://togithub.com/mermaid-js/mermaid) | [`10.7.0` ->
`10.8.0`](https://renovatebot.com/diffs/npm/mermaid/10.7.0/10.8.0) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/mermaid/10.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/mermaid/10.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/mermaid/10.7.0/10.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/mermaid/10.7.0/10.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>mermaid-js/mermaid (mermaid)</summary>

###
[`v10.8.0`](https://togithub.com/mermaid-js/mermaid/releases/tag/v10.8.0)

[Compare
Source](https://togithub.com/mermaid-js/mermaid/compare/v10.7.0...v10.8.0)

### v10.8.0

#### Features

<img width="375" alt="image"
src="https://github.com/mermaid-js/mermaid/assets/5837277/c17405fb-2a06-4f7c-9c77-5eaba6c76747">

- Adding new diagram type - Block Diagram by
[@&#8203;knsv](https://togithub.com/knsv) in
[https://github.com/mermaid-js/mermaid/pull/5221](https://togithub.com/mermaid-js/mermaid/pull/5221)

- Feature/5114 add parallel commit config by
[@&#8203;mathbraga](https://togithub.com/mathbraga) in
[https://github.com/mermaid-js/mermaid/pull/5161](https://togithub.com/mermaid-js/mermaid/pull/5161)

- Changes to Gantt Parsers to allow hashes and semicolons to titles,
sections, and task data. by
[@&#8203;FutzMonitor](https://togithub.com/FutzMonitor) in
[https://github.com/mermaid-js/mermaid/pull/5095](https://togithub.com/mermaid-js/mermaid/pull/5095)

- Feature/4653 add actor-top class to sequence diagram by
[@&#8203;Ronid1](https://togithub.com/Ronid1) in
[https://github.com/mermaid-js/mermaid/pull/5241](https://togithub.com/mermaid-js/mermaid/pull/5241)

#### Documentation

- Updated gantt chart docs to show all config options by
[@&#8203;murdoa](https://togithub.com/murdoa) in
[https://github.com/mermaid-js/mermaid/pull/5192](https://togithub.com/mermaid-js/mermaid/pull/5192)
- Contribution documentation improvements by
[@&#8203;nirname](https://togithub.com/nirname) in
[https://github.com/mermaid-js/mermaid/pull/5132](https://togithub.com/mermaid-js/mermaid/pull/5132)
- Update flowchart.md - how to use font-awesome
[#&#8203;5195](https://togithub.com/mermaid-js/mermaid/issues/5195) by
[@&#8203;arukiidou](https://togithub.com/arukiidou) in
[https://github.com/mermaid-js/mermaid/pull/5196](https://togithub.com/mermaid-js/mermaid/pull/5196)
- Add more detailed docs for Gantt tasks by
[@&#8203;sorenisanerd](https://togithub.com/sorenisanerd) in
[https://github.com/mermaid-js/mermaid/pull/5194](https://togithub.com/mermaid-js/mermaid/pull/5194)
- Docs/4974 reorder integration links by
[@&#8203;Ronid1](https://togithub.com/Ronid1) in
[https://github.com/mermaid-js/mermaid/pull/5066](https://togithub.com/mermaid-js/mermaid/pull/5066)
- docs: fix swimm link by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[https://github.com/mermaid-js/mermaid/pull/5219](https://togithub.com/mermaid-js/mermaid/pull/5219)
- Update Slack community links to Discord by
[@&#8203;Olegt0rr](https://togithub.com/Olegt0rr) in
[https://github.com/mermaid-js/mermaid/pull/5225](https://togithub.com/mermaid-js/mermaid/pull/5225)
- Docs: Mermaid chart updates by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[https://github.com/mermaid-js/mermaid/pull/5232](https://togithub.com/mermaid-js/mermaid/pull/5232)
- Fix typos in timeline syntax samples by
[@&#8203;sblom](https://togithub.com/sblom) in
[https://github.com/mermaid-js/mermaid/pull/5139](https://togithub.com/mermaid-js/mermaid/pull/5139)

#### Bug fixes

- Bug/5059 fix external connection after updating edges by
[@&#8203;mathbraga](https://togithub.com/mathbraga) in
[https://github.com/mermaid-js/mermaid/pull/5127](https://togithub.com/mermaid-js/mermaid/pull/5127)
- \[Fix] Sequence diagram actor menu popup by
[@&#8203;vitorsss](https://togithub.com/vitorsss) in
[https://github.com/mermaid-js/mermaid/pull/5160](https://togithub.com/mermaid-js/mermaid/pull/5160)
- fix: Dompurify Hooks by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5236](https://togithub.com/mermaid-js/mermaid/pull/5236)
- Accurate pie chart labeling for text alignment by
[@&#8203;JenningsWilliam](https://togithub.com/JenningsWilliam) in
[https://github.com/mermaid-js/mermaid/pull/5141](https://togithub.com/mermaid-js/mermaid/pull/5141)
- fix: Redirect of old URLs by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5250](https://togithub.com/mermaid-js/mermaid/pull/5250)
- Fixed Typo in ErrorRenderer.ts by
[@&#8203;FutzMonitor](https://togithub.com/FutzMonitor) in
[https://github.com/mermaid-js/mermaid/pull/5256](https://togithub.com/mermaid-js/mermaid/pull/5256)

#### Chores

- Revert "Revert 5041 feature/4935 subgraph title margin config option"
by [@&#8203;mathbraga](https://togithub.com/mathbraga) in
[https://github.com/mermaid-js/mermaid/pull/5205](https://togithub.com/mermaid-js/mermaid/pull/5205)
- build(deps-dev): bump follow-redirects from 1.15.2 to 1.15.5 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/mermaid-js/mermaid/pull/5200](https://togithub.com/mermaid-js/mermaid/pull/5200)
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[https://github.com/mermaid-js/mermaid/pull/5150](https://togithub.com/mermaid-js/mermaid/pull/5150)
- E2E Image comparison by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5208](https://togithub.com/mermaid-js/mermaid/pull/5208)
- E2E test by [@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5210](https://togithub.com/mermaid-js/mermaid/pull/5210)
- Optimise caching of test results by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5213](https://togithub.com/mermaid-js/mermaid/pull/5213)
- Update update-browserlist.yml to fix deprecation and action fails by
[@&#8203;Abrifq](https://togithub.com/Abrifq) in
[https://github.com/mermaid-js/mermaid/pull/5151](https://togithub.com/mermaid-js/mermaid/pull/5151)
- UpdateCypress by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5228](https://togithub.com/mermaid-js/mermaid/pull/5228)
- Use node v20 by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5248](https://togithub.com/mermaid-js/mermaid/pull/5248)
- Convert Mindmap to TS by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5247](https://togithub.com/mermaid-js/mermaid/pull/5247)
- chore: Add interface naming Convention by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[https://github.com/mermaid-js/mermaid/pull/5254](https://togithub.com/mermaid-js/mermaid/pull/5254)

#### New Contributors

- [@&#8203;murdoa](https://togithub.com/murdoa) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/5192](https://togithub.com/mermaid-js/mermaid/pull/5192)
- [@&#8203;arukiidou](https://togithub.com/arukiidou) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/5196](https://togithub.com/mermaid-js/mermaid/pull/5196)
- [@&#8203;sorenisanerd](https://togithub.com/sorenisanerd) made their
first contribution in
[https://github.com/mermaid-js/mermaid/pull/5194](https://togithub.com/mermaid-js/mermaid/pull/5194)
- [@&#8203;Ronid1](https://togithub.com/Ronid1) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/5066](https://togithub.com/mermaid-js/mermaid/pull/5066)
- [@&#8203;Olegt0rr](https://togithub.com/Olegt0rr) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/5225](https://togithub.com/mermaid-js/mermaid/pull/5225)
- [@&#8203;vitorsss](https://togithub.com/vitorsss) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/5160](https://togithub.com/mermaid-js/mermaid/pull/5160)
- [@&#8203;sblom](https://togithub.com/sblom) made their first
contribution in
[https://github.com/mermaid-js/mermaid/pull/5139](https://togithub.com/mermaid-js/mermaid/pull/5139)
- [@&#8203;JenningsWilliam](https://togithub.com/JenningsWilliam) made
their first contribution in
[https://github.com/mermaid-js/mermaid/pull/5141](https://togithub.com/mermaid-js/mermaid/pull/5141)

**Full Changelog**:
mermaid-js/mermaid@v10.7.0...v10.8.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/levaintech/contented).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNTMuMiIsInVwZGF0ZWRJblZlciI6IjM3LjE1My4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
pcolby added a commit to pcolby/ww that referenced this pull request Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use # or ; in gantt diagram
4 participants