Add Support for Highlighting Multiple Words in the Same Line #2059

zlimez · 2022-12-17T13:39:28Z

What is the purpose of this pull request?

Overview of changes:

Fixes #2050

Suspect this is the block of code preventing multiple words to be highlighted separately on the same line, as only the first highlight rule is applied.

Modified to collate all rules that is applied to the current line, then apply them as a batch.

Anything you'd like to highlight/discuss:

Overlapping highlight intervals are combined and processed in helper.js collateAllIntervals(boundaries) function. Extract applyHighlight method from 'HighlightRule' to Highlighter. Not certain if that is the best approach.

Highlight line takes precedence over highlight whole text over highlight partial text when applied on the same line. Again, not sure if this is the desirable behaviour.

Testing instructions:

Edits made to testCodeBlocks.md:

Modified the fence code block test cases partial word variant, full word variant, partial character variant and full word variant to include situations where more than one highlight rules is applied on one line.
Included a fence code block test case under all attr should behave as expected, that does a combined testing of all highlight rules, with some overlapping bounds.

Sample output

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No unrelated changes

packages/cli/test/functional/test_site/expected/testCodeBlocks.html

packages/core/src/lib/markdown-it/highlight/HighlightRule.js

packages/core/src/lib/markdown-it/index.js

packages/core/src/lib/markdown-it/highlight/Highlighter.js

packages/core/src/lib/markdown-it/highlight/helper.js

packages/core/src/lib/markdown-it/index.js

zlimez · 2023-02-13T07:05:46Z

Hi @zlimez, conflicting files due to recent updates to the highlight parsing. Checking if you are still keen to implement the last few comments/fixes as requested?

cc @yucheng11122017 @lhw-1 you guys can keep tabs on this and render help if needed (given your recent involvement in changes wrt code highlight, thanks!)

Yap, I am still interested but been really busy please give me a week or so, thx!!

yucheng11122017 · 2023-07-18T06:51:28Z

Hi @zlimez, are you still keen to work on this PR? No pressure if you arent

zlimez · 2023-07-25T05:25:47Z

Hi @zlimez, are you still keen to work on this PR? No pressure if you arent

Sorry I completely forgot about the existence of this PR, can I have till end of this week to fast forward and update the documentations?

yucheng11122017 · 2023-07-25T15:39:44Z

Hi @zlimez, are you still keen to work on this PR? No pressure if you arent

Sorry I completely forgot about the existence of this PR, can I have till end of this week to fast forward and update the documentations?

Yup sure! Feel free to reach out if there is any issues with merge conflicts as these files have been since converted to typescript

zlimez · 2023-08-04T15:47:06Z

I have added documentations and migrated to TS. Please help me review and soz for taking so long.

zlimez · 2023-08-10T01:12:56Z

Hi @zlimez, are you still keen to work on this PR? No pressure if you arent

Sorry I completely forgot about the existence of this PR, can I have till end of this week to fast forward and update the documentations?

Yup sure! Feel free to reach out if there is any issues with merge conflicts as these files have been since converted to typescript

Hi hi, as per my latest comment I am done with the migration and docs when you are free please kindly help me review 🙂

jovyntls

Thanks for working on this @zlimez!

Haven't tested the functionality as I'm lacking context on this issue but added some comments on the code :)

packages/core/src/lib/markdown-it/highlight/helper.ts

packages/core/src/lib/markdown-it/highlight/Highlighter.ts

packages/core/src/lib/markdown-it/index.ts

packages/core/src/lib/markdown-it/highlight/HighlightRuleComponent.ts

packages/core/test/unit/lib/markdown-it/highlight/helper.test.ts

packages/core/test/unit/lib/markdown-it/highlight/HighlightRuleComponent.test.ts

packages/core/src/lib/markdown-it/highlight/HighlightRule.ts

tlylt · 2023-11-29T16:00:47Z

@zlimez sorry for the delay, review is underway. If you could spare some time to revisit this PR, please see if there are readability fixes that you can apply. Thanks.

zlimez · 2023-11-30T11:04:25Z

@zlimez sorry for the delay, review is underway. If you could spare some time to revisit this PR, please see if there are readability fixes that you can apply. Thanks.

Ok I will take a look. Thanks.

tlylt

Hi @zlimez, just a few more comments to address and I think it should be good to merge soon.

packages/core/src/lib/markdown-it/index.ts

packages/core/test/unit/lib/markdown-it/highlight/HightlightRule.test.ts

packages/core/src/lib/markdown-it/highlight/HighlightRule.ts

packages/core/src/lib/markdown-it/highlight/HighlightRuleComponent.ts

zlimez · 2023-12-03T14:20:07Z

I have extracted the line number check as per requested hope it is alright

packages/core/src/lib/markdown-it/highlight/HighlightRuleComponent.ts

packages/core/src/lib/markdown-it/highlight/HighlightRule.ts

tlylt

LGTM

tlylt · 2023-12-10T13:27:02Z

@all-contributors please add @zlimez for code

allcontributors · 2023-12-10T13:27:13Z

@tlylt

I've put up a pull request to add @zlimez! 🎉

zlimez · 2023-12-10T15:30:15Z

LGTM

Yeah thx for helping me review for so long ><

This comment was marked as resolved.

Sign in to view

tlylt self-requested a review December 26, 2022 02:41

ang-zeyu reviewed Dec 27, 2022

View reviewed changes

packages/core/src/lib/markdown-it/highlight/helper.js Outdated Show resolved Hide resolved

tlylt reviewed Dec 28, 2022

View reviewed changes

packages/core/src/lib/markdown-it/index.js Outdated Show resolved Hide resolved

packages/core/src/lib/markdown-it/index.js Outdated Show resolved Hide resolved

packages/core/src/lib/markdown-it/index.js Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

tlylt marked this pull request as draft April 6, 2023 06:21

ang-zeyu mentioned this pull request Jun 21, 2023

Added List, ListItem, and Media Features - Resolves #906 and #2272 #2309

Closed

6 tasks

zlimez added 5 commits August 4, 2023 19:40

Migrate to typescript

8b28c82

Add docs

359e148

Fix lint errors

ede79ec

Remove empty rules

df1e667

Add tests

aad2d49

zlimez force-pushed the fix_2050 branch from 85387b4 to aad2d49 Compare August 4, 2023 15:45

zlimez marked this pull request as ready for review August 4, 2023 15:46

Merge branch 'master' into fix_2050

611da71

jovyntls reviewed Aug 14, 2023

View reviewed changes

packages/core/src/lib/markdown-it/highlight/Highlighter.ts Outdated Show resolved Hide resolved

packages/core/src/lib/markdown-it/index.ts Outdated Show resolved Hide resolved

jovyntls reviewed Aug 14, 2023

View reviewed changes

packages/core/src/lib/markdown-it/index.ts Outdated Show resolved Hide resolved

packages/core/src/lib/markdown-it/index.ts Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

zlimez commented Nov 26, 2023

View reviewed changes

packages/core/src/lib/markdown-it/highlight/HighlightRuleComponent.ts Show resolved Hide resolved

zlimez commented Nov 26, 2023

View reviewed changes

packages/core/src/lib/markdown-it/highlight/HighlightRuleComponent.ts Outdated Show resolved Hide resolved

tlylt requested changes Nov 27, 2023

View reviewed changes

formatting

1910332

zlimez force-pushed the fix_2050 branch from ed334db to 8b94e7a Compare November 27, 2023 11:22

Merge remote-tracking branch 'origin/fix_2050'

dc0f623

zlimez force-pushed the fix_2050 branch from 8b94e7a to dc0f623 Compare November 27, 2023 15:05

tlylt reviewed Nov 28, 2023

View reviewed changes

packages/core/src/lib/markdown-it/highlight/HighlightRule.ts Show resolved Hide resolved

packages/core/src/lib/markdown-it/highlight/HighlightRule.ts Show resolved Hide resolved

restore comments

4a11c05

add highlight rule priority comments as requested

38a27f4

tlylt requested changes Dec 3, 2023

View reviewed changes

Add comments and extract line num check

c589486

tlylt reviewed Dec 3, 2023

View reviewed changes

packages/core/src/lib/markdown-it/highlight/HighlightRuleComponent.ts Show resolved Hide resolved

tlylt reviewed Dec 3, 2023

View reviewed changes

packages/core/src/lib/markdown-it/highlight/HighlightRule.ts Show resolved Hide resolved

zlimez and others added 4 commits December 4, 2023 15:55

Add test cases for highlightRule

81ae091

One more tc

e7e170a

Remove empty line from token content (highlighting) + update test cases

3f7008b

add unit test

57c3f3f

tlylt approved these changes Dec 10, 2023

View reviewed changes

tlylt added this to the v5.2.0 milestone Dec 10, 2023

tlylt added the r.Minor Version resolver: increment by 0.1.0 label Dec 10, 2023

tlylt changed the title ~~Update highlight parsing~~ Add Support for Highlighting Multiple Words in the Same Line Dec 10, 2023

tlylt merged commit 11702c9 into MarkBind:master Dec 10, 2023
8 checks passed

allcontributors bot mentioned this pull request Dec 10, 2023

add zlimez as a contributor for code #2381

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Support for Highlighting Multiple Words in the Same Line #2059

Add Support for Highlighting Multiple Words in the Same Line #2059

zlimez commented Dec 17, 2022 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

zlimez commented Feb 13, 2023

yucheng11122017 commented Jul 18, 2023

zlimez commented Jul 25, 2023

yucheng11122017 commented Jul 25, 2023

zlimez commented Aug 4, 2023

zlimez commented Aug 10, 2023

jovyntls left a comment •

edited

Loading

This comment was marked as resolved.

tlylt commented Nov 29, 2023

zlimez commented Nov 30, 2023

tlylt left a comment

zlimez commented Dec 3, 2023

tlylt left a comment

tlylt commented Dec 10, 2023

allcontributors bot commented Dec 10, 2023

zlimez commented Dec 10, 2023

Add Support for Highlighting Multiple Words in the Same Line #2059

Add Support for Highlighting Multiple Words in the Same Line #2059

Conversation

zlimez commented Dec 17, 2022 • edited Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

zlimez commented Feb 13, 2023

yucheng11122017 commented Jul 18, 2023

zlimez commented Jul 25, 2023

yucheng11122017 commented Jul 25, 2023

zlimez commented Aug 4, 2023

zlimez commented Aug 10, 2023

jovyntls left a comment • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

tlylt commented Nov 29, 2023

zlimez commented Nov 30, 2023

tlylt left a comment

Choose a reason for hiding this comment

zlimez commented Dec 3, 2023

tlylt left a comment

Choose a reason for hiding this comment

tlylt commented Dec 10, 2023

allcontributors bot commented Dec 10, 2023

zlimez commented Dec 10, 2023

zlimez commented Dec 17, 2022 •

edited

Loading

jovyntls left a comment •

edited

Loading