Better whitespace handling, parse and keep where allowed #2720
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adjusted test cases to test whitespace characters are not touched. Signed-off-by: Ferdinand Thiessen <[email protected]>
Switch to @hedgedoc/markdown-it-task-lists to fix parsing of task lists, namely allowing any whitespace inside the brackets, as by the github markdown specs. Doing this as markdown-it-task-lists seems orphaned, while hedgedoc is maintained and the bug is fixed there. This required adjusting the task-list testcase as the output is slightly different. Signed-off-by: Ferdinand Thiessen <[email protected]>
Contributor
Author
|
/compile amend |
This was referenced Jul 15, 2022
Signed-off-by: Ferdinand Thiessen <[email protected]> Signed-off-by: nextcloud-command <[email protected]>
nextcloud-command
force-pushed
the
keep-whitespaces
branch
from
c9b439d to
75cc0b8
Compare
juliusknorr
approved these changes
Jul 15, 2022
Member
juliusknorr
left a comment
juliusknorr
left a comment
There was a problem hiding this comment.
Very nice, thanks for the effort on this 👍
vinicius73
reviewed
Jul 15, 2022
| * @param {string} cls Class name to query | ||
| */ | ||
| function includesClass(token, cls) { | ||
| return token.attrGet('class')?.split(' ').includes(cls) || false |
Member
There was a problem hiding this comment.
We can avoid .split(' ')
https://developer.mozilla.org/pt-BR/docs/Web/JavaScript/Reference/Global_Objects/String/includes
Contributor
Author
There was a problem hiding this comment.
Not realy as that would match if the queried class name is part of another class, consider this:
token = {
class: "foo-bar left"
}
includesClass(token, "bar")without the split this would return true even if bar is not a class of that token.
Member
|
I've left just one small comment. |
vinicius73
approved these changes
Jul 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Ensure whitespaces in markdown files are preserved where possible,
as mentioned in the issues it is quite annoying to have
textto remove all formatting like new lines and additional spaces.Even if CommonMark allows them they are currently removed as TipTap collapses them (by default) like HTML does it.
So TipTap is now configured to preserve whitespaces where possible.
Also this includes task lists, as the github markdown guides explictily allow any whitespace inside the brackets,
but the current
markdown-itextension did not.So switch to
@hedgedoc/markdown-it-task-liststo fix this parsing of task lists, namely allowing any whitespaceinside the brackets, as by the github markdown specs.
Doing this as markdown-it-task-lists seems orphaned, while hedgedoc is maintained and the bug is fixed there.
This required adjusting the task-list testcase as the output is slightly different.