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

Split formatted markdown strings with unicode support. #4470

Merged
merged 29 commits into from Jul 25, 2023
Merged

Conversation

sidharthv96
Copy link
Member

@sidharthv96 sidharthv96 commented Jun 9, 2023

📑 Summary

Continuation of #4416

📏 Design Decisions

This will handle graphemes using Intl.Segmenter if available. (Not available in FF)
It also handles lines word by word instead of per character. So performance should be better as we are doing a lot less DOM operations.

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added unit/e2e tests (if appropriate)
  • 📓 have added documentation (if appropriate)
  • 🔖 targeted develop branch

sidharthv96 and others added 8 commits June 9, 2023 11:06
* develop:
  Update docs
  Rename info to note
  Rename "info" to "note"
  Update all patch dependencies
  Fix Directives Documentation
  Update tutorial link
  Run build
  Fix link to Tutorials from n00b-overview page
  UPdated version to 10.2.3
  Remove old changelog
  Remove old changelog
  Setting version to 10.2.2
  #4446 Improved regular expression
  #4446 Updating the cleanup criteria
  #4438 Reverted to the changes from #4285
  Fix download
  Fix compile error in docs.
  Fix Contributor link in homepage
  Update docs
  Add hint on "flowchart" and "graph" (and some more styling)
@sidharthv96 sidharthv96 marked this pull request as draft July 6, 2023 04:11
* develop: (293 commits)
  chore: Remove lint warnings in example-diagram
  chore: Reduce codecov pushes
  Turn off codecov project status check
  build(docs): fix links to `config.schema.json`
  ci(lint): check if MermaidConfig types are in sync
  docs: add link to mermaid config docs in sidebar
  test(config): add temp test for defaultConfig
  build(types): create types from config JSON Schema
  build(types): add script to generate Config types
  build(docs): build JSON Schema docs
  build: use vite to get default mermaid config
  feat: add Mermaid Config in JSON Schema format
  docs: add support for ```regexp``` code blocks
  test: test partial QuadrantChartConfig options
  test: fix types in `config.spec.ts`
  style: fix lint issues in src/config.spec.ts
  test: rename src/config.spec.js to config.spec.ts
  fix(quadrant): make quadrant options optional
  fix lint
  update homepage community link
  ...
@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #4470 (651bc98) into develop (509a580) will increase coverage by 3.10%.
The diff coverage is 69.23%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4470      +/-   ##
===========================================
+ Coverage    73.27%   76.37%   +3.10%     
===========================================
  Files          143      135       -8     
  Lines        14535    13764     -771     
  Branches       516      550      +34     
===========================================
- Hits         10650    10512     -138     
+ Misses        3774     3143     -631     
+ Partials       111      109       -2     
Flag Coverage Δ
e2e 83.59% <69.23%> (+4.76%) ⬆️
unit 45.32% <ø> (ø)

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

Files Changed Coverage Δ
packages/mermaid/src/rendering-util/splitText.ts 62.74% <62.74%> (ø)
packages/mermaid/src/rendering-util/createText.ts 87.34% <91.66%> (ø)
...mermaid/src/rendering-util/handle-markdown-text.ts 97.22% <100.00%> (ø)

... and 34 files with indirect coverage changes

@sidharthv96 sidharthv96 marked this pull request as ready for review July 6, 2023 16:07
@sidharthv96
Copy link
Member Author

Review request: @MikeJeffers :)

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.

I do not fully understand what is happening here, I'm relatively new to the code base, so I suppose I cannot submit reasonable review for it

packages/mermaid/src/rendering-util/createText.ts Outdated Show resolved Hide resolved
Copy link
Member

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

This isn't a full review right now, since I don't have much time, but from what I've seen so far, this PR looks great 👍.

However, there's one major flaw: there's no tests for environments where Intl.Segmenter is not available (very important since Firefox doesn't support it).

In order to fix this, can we somehow mock that Intl.Segmenter doesn't exist in Vitest and/or Cypress?

Or, we could just make Cypress run in the Firefox browser too, but that will make our E2E tests very very slow.

@sidharthv96 sidharthv96 changed the title Split unicode Split formatted markdown strings with unicode support. Jul 6, 2023
Copy link
Contributor

@MikeJeffers MikeJeffers left a comment

Choose a reason for hiding this comment

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

Speaking to the improvements over the original fix - I approve and appreciate your work improving the initial fix! But I don't want to overstep in prematurely approving the whole PR as a lot here has changed that others might need to speak to first.

Copy link
Member

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

Thanks for adding some tests for when Intl.Segmenter isn't available! I agree, the Cypress tests would be nice to have, but having the tests in Vitest is good enough to get some code-coverage!

One last thing, it might be worth adding some info in the comments in functions that use Intl.Segmenter, since they use the default locale, which may return slightly different results on some locales (e.g. Bengali).

packages/mermaid/src/rendering-util/splitText.spec.ts Outdated Show resolved Hide resolved
packages/mermaid/src/rendering-util/splitText.ts Outdated Show resolved Hide resolved
Co-authored-by: Alois Klink <alois@aloisklink.com>
@Yokozuna59
Copy link
Member

Yokozuna59 commented Jul 23, 2023

Shouldn't this also resolve #1900 and #1925?

@sidharthv96
Copy link
Member Author

Shouldn't this also resolve #1900 and #1925?

These seems to be fixed already.

sidharthv96 and others added 2 commits July 25, 2023 19:08
Already checked in lint.
Co-authored-by: Alois Klink <alois@aloisklink.com>
…maid into sidv/splitUnicode

* 'sidv/splitUnicode' of https://github.com/mermaid-js/mermaid:
  Update packages/mermaid/src/rendering-util/types.d.ts
@knsv knsv added this pull request to the merge queue Jul 25, 2023
Merged via the queue into develop with commit c99e1c6 Jul 25, 2023
20 of 21 checks passed
github-merge-queue bot pushed a commit to fuxingloh/contented that referenced this pull request Jul 31, 2023
[![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.2.4` ->
`10.3.0`](https://renovatebot.com/diffs/npm/mermaid/10.2.4/10.3.0) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/mermaid/10.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/mermaid/10.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/mermaid/10.2.4/10.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/mermaid/10.2.4/10.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

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

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

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

#### What's Changed

##### Features

- Sankey diagrams by [@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4502
- Feature/1838 actor creation destruction by
[@&#8203;Valentine14th](https://togithub.com/Valentine14th) in
[mermaid-js/mermaid#4466
- Vertical branches in Git Diagram by
[@&#8203;mastersibin](https://togithub.com/mastersibin) in
[mermaid-js/mermaid#4639
- Use JSON Schema to define and document `MermaidConfig` by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4112
- Remove the test checking whether the JSON Schema default config
matched the old default config by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4610
- Fixes support of the macro `ContainerQueue_Ext` for C4 diagrams
definition. by [@&#8203;kislerdm](https://togithub.com/kislerdm) in
[mermaid-js/mermaid#4577

##### Bugfixes

- Make quadrant chart options TypeScript types optional by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4602
- Remove double parsing by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4587
- Fix flowchart tooltip typing bug by
[@&#8203;lishid](https://togithub.com/lishid) in
[mermaid-js/mermaid#4562
- Bug/4590 allow notes identical to keywords by
[@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) in
[mermaid-js/mermaid#4597
- feat: allow specifying on which weekday a tickInterval should start by
[@&#8203;leinelissen](https://togithub.com/leinelissen) in
[mermaid-js/mermaid#4634
- Split formatted markdown strings with unicode support. by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4470
- fix: Mind maps handles `-` signs in node ids/text by
[@&#8203;knsv](https://togithub.com/knsv)

##### Chores

- Remove all TypeScript enums and forbid them in ESLint by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4580
- refactor accessibility by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[mermaid-js/mermaid#4551
- chore: Reduce codecov pushes by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4604
- Run PR-labeler-config-validator only if config changes by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4607
- chore(deps): update all minor dependencies (minor) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4624
- Update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4566
- Update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4581
- Rename workflow jobs by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4574
- Removed unused code in state diagrams by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4631
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4623
- chore: remove unused `devDependency` on coveralls by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4641
- Allow entity diagram attribute names to start with asterisk by
[@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) in
[mermaid-js/mermaid#4588
- Bug/4592 fix new line padding class diagram by
[@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) in
[mermaid-js/mermaid#4633
- Fix graph not loading when the img loads too fast or fail to load by
[@&#8203;pierrickouw](https://togithub.com/pierrickouw) in
[mermaid-js/mermaid#4496
- convert `cypress/helpers/util.js` to ts by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[mermaid-js/mermaid#4552
- build(deps-dev): bump word-wrap from 1.2.3 to 1.2.4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[mermaid-js/mermaid#4652
- chore(deps): update all minor dependencies (minor) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4663
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4662

##### Documentation

- Sankey: Remove duplicated examples by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4595
- Release docs by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4493
- Update latest news section by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#4495
- Fix Typo by [@&#8203;ryru](https://togithub.com/ryru) in
[mermaid-js/mermaid#4567
- Docs: add ChatGPT plugin blog post by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#4570
- Fix relative link to theme variables list by
[@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) in
[mermaid-js/mermaid#4573
- Fix docs:dev by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4598
- Docs: update link - "Join the Community" by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#4601
- Support docs:dev in docker by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4599
- docs(flowchart): add documentation on multiple nodes style by
[@&#8203;tomperr](https://togithub.com/tomperr) in
[mermaid-js/mermaid#4600
- Avoid downloading avtars everytime on docs:dev by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4603
- docs: Fix checkbox syntax by
[@&#8203;guilhermgonzaga](https://togithub.com/guilhermgonzaga) in
[mermaid-js/mermaid#4646
- Fix the "Edit this page on GitHub" link in Vitepress documentation for
the Mermaid Config pages by
[@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4640
- Support MERMAID_RELEASE_VERSION in docs. by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4612
- Docs: update Latest News section by
[@&#8203;huynhicode](https://togithub.com/huynhicode) in
[mermaid-js/mermaid#4655
- added Typora to integrations list by
[@&#8203;kgilbert78](https://togithub.com/kgilbert78) in
[mermaid-js/mermaid#4666
- Docs: Corrects name of C4 link by
[@&#8203;Incognito](https://togithub.com/Incognito) in
[mermaid-js/mermaid#4660
- Fix a typo by [@&#8203;gjtorikian](https://togithub.com/gjtorikian) in
[mermaid-js/mermaid#4396

#### New Contributors

- [@&#8203;ryru](https://togithub.com/ryru) made their first
contribution in
[mermaid-js/mermaid#4567
- [@&#8203;ibrahimWassouf](https://togithub.com/ibrahimWassouf) made
their first contribution in
[mermaid-js/mermaid#4573
- [@&#8203;kislerdm](https://togithub.com/kislerdm) made their first
contribution in
[mermaid-js/mermaid#4577
- [@&#8203;leinelissen](https://togithub.com/leinelissen) made their
first contribution in
[mermaid-js/mermaid#4634
- [@&#8203;pierrickouw](https://togithub.com/pierrickouw) made their
first contribution in
[mermaid-js/mermaid#4496
- [@&#8203;mastersibin](https://togithub.com/mastersibin) made their
first contribution in
[mermaid-js/mermaid#4639
- [@&#8203;kgilbert78](https://togithub.com/kgilbert78) made their first
contribution in
[mermaid-js/mermaid#4666
- [@&#8203;Incognito](https://togithub.com/Incognito) made their first
contribution in
[mermaid-js/mermaid#4660
- [@&#8203;gjtorikian](https://togithub.com/gjtorikian) made their first
contribution in
[mermaid-js/mermaid#4396

**Full Changelog**:
mermaid-js/mermaid@v10.2.4...v10.3.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:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMS4wIiwidXBkYXRlZEluVmVyIjoiMzYuMTEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@Yokozuna59 Yokozuna59 deleted the sidv/splitUnicode branch August 11, 2023 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

timeling bug for chinese Properly handle unicode when splitting words
6 participants