-
Notifications
You must be signed in to change notification settings - Fork 97
Fix applying styles for inline code syntaxes #732
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
Fix applying styles for inline code syntaxes #732
Conversation
jmusial
left a comment
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.
LGTM ![]()
src/rangeUtils.ts
Outdated
| * This includes emojis and syntaxes of inline code blocks. | ||
| */ | ||
| function getRangesToExcludeFormatting(ranges: MarkdownRange[]) { | ||
| function getRangesToExcludeFormatting(ranges: MarkdownRange[]): ExtendedMarkdownRange[] { |
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.
NAB but i think this should do and you can get rid of #71 then
| function getRangesToExcludeFormatting(ranges: MarkdownRange[]): ExtendedMarkdownRange[] { | |
| function getRangesToExcludeFormatting(ranges: ExtendedMarkdownRange[]): ExtendedMarkdownRange[] { |
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.
If I do that, I will get eslintno-param-reassign. So we need to keep #71. Removing as doesn't seem to me to be worthy, because this function should transform MarkdownRange to ExtendedMarkdownRange. It's more logical to me in that way
| import {expect} from '@jest/globals'; | ||
| import type {MarkdownRange} from '../commonTypes'; | ||
| import parseExpensiMark from '../parseExpensiMark'; | ||
| import type {ExtendedMarkdownRange} from '../rangeUtils'; |
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.
Not a huge fan of ExtendedMarkdownRange, why do we need this? Why can't we just extend the existing MarkdownRange type?
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.
In fact, we can 🤔 Changed that in 0169cdc
Details
This is a follow-up PR to the previous fix for cursor positioning in inline code markdown. Here, I introduced a mechanism that marks opening and closing syntax, and based on that, it extends the range before opening syntax and the range after closing syntax. Thanks to that, styles are continuous
Related Issues
Expensify/App#69493
Manual Tests
Linked PRs
#729