Skip to content

Better whitespace handling, parse and keep where allowed #2720

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.

Sign up for GitHub

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

Merged
vinicius73 merged 3 commits into from
Jul 15, 2022
Merged

Better whitespace handling, parse and keep where allowed #2720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1b7d876
Ensure whitespaces in markdown files are preserved
susnux Jul 9, 2022
613b689
Fix parsing of whitespace used in task lists
susnux Jul 15, 2022
75cc0b8
Compile assets
susnux Jul 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions js/editor.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/editor.js.map
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-files.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-files.js.map
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-public.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-public.js.map
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-text.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-text.js.map
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-viewer.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-viewer.js.map
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/vendors.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/vendors.js.map
View file
Open in desktop

Large diffs are not rendered by default.

26 changes: 15 additions & 11 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
],
"dependencies": {
"@_ueberdosis/prosemirror-tables": "^1.1.3",
"@hedgedoc/markdown-it-task-lists": "^1.0.5",
"@nextcloud/auth": "^1.3.0",
"@nextcloud/axios": "^1.10.0",
"@nextcloud/browser-storage": "^0.1.1",
Expand Down Expand Up @@ -78,7 +79,6 @@
"@tiptap/vue-2": "^2.0.0-beta.84",
"markdown-it": "^13.0.0",
"markdown-it-container": "^3.0.0",
"markdown-it-task-lists": "^2.1.1",
"path-normalize": "^6.0.6",
"prosemirror-collab": "^1.3.0",
"prosemirror-inputrules": "^1.2.0",
Expand Down
3 changes: 2 additions & 1 deletion src/extensions/HardBreak.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ const HardBreak = TipTapHardBreak.extend({
toMarkdown(state, node, parent, index) {
for (let i = index + 1; i < parent.childCount; i++) {
if (parent.child(i).type !== node.type) {
state.write(' \n')
if (parent.child(i).text?.startsWith('\n')) state.write(' ')
else state.write(' \n')
return
}
}
Expand Down
8 changes: 7 additions & 1 deletion src/extensions/RichText.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { Extension } from '@tiptap/core'

/* eslint-disable import/no-named-as-default */
import Document from '@tiptap/extension-document'
import Paragraph from '@tiptap/extension-paragraph'
import TipTapParagraph from '@tiptap/extension-paragraph'
import Text from '@tiptap/extension-text'
import Blockquote from '@tiptap/extension-blockquote'
import OrderedList from '@tiptap/extension-ordered-list'
Expand All @@ -45,6 +45,12 @@ import Callout from './../nodes/Callouts.js'

import { Strong, Italic, Strike, Link, Underline } from './../marks/index.js'

const Paragraph = TipTapParagraph.extend({
parseHTML() {
return this.parent().map(rule => Object.assign(rule, { preserveWhitespace: 'full' }))
},
})

export default Extension.create({
name: 'RichText',

Expand Down
2 changes: 1 addition & 1 deletion src/markdownit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import MarkdownIt from 'markdown-it'
import taskLists from 'markdown-it-task-lists'
import taskLists from '@hedgedoc/markdown-it-task-lists'
import underline from './underline.js'
import splitMixedLists from './splitMixedLists.js'
import callouts from './callouts.js'
Expand Down
16 changes: 12 additions & 4 deletions src/markdownit/splitMixedLists.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@
* @param {object} md Markdown object
*/
export default function splitMixedLists(md) {
md.core.ruler.after('github-task-lists', 'split-mixed-task-lists', state => {
md.core.ruler.after('task-lists', 'split-mixed-task-lists', state => {
const tokens = state.tokens

for (let i = 0; i < tokens.length; i++) {
const token = tokens[i]
if (token.attrGet('class') !== 'contains-task-list') {
if (!includesClass(token, 'contains-task-list')) {
continue
}
const firstChild = tokens[i + 1]
const startsWithTask = firstChild.attrGet('class') === 'task-list-item'
const startsWithTask = includesClass(firstChild, 'task-list-item')
if (!startsWithTask) {
token.attrs.splice(token.attrIndex('class'))
if (token.attrs.length === 0) {
Expand All @@ -42,7 +42,7 @@ export default function splitMixedLists(md) {
}
const splitBefore = findChildOf(tokens, i, child => {
return child.nesting === 1
&& child.attrGet('class') !== firstChild.attrGet('class')
&& includesClass(child, 'task-list-item') !== startsWithTask
})
if (splitBefore > i) {
splitListAt(tokens, splitBefore, state.Token)
Expand All @@ -53,6 +53,14 @@ export default function splitMixedLists(md) {
})
}

/**
* @param {object} token MarkdownIT token
* @param {string} cls Class name to query
*/
function includesClass(token, cls) {
return token.attrGet('class')?.split(' ').includes(cls) || false
Copy link
Member

@vinicius73 vinicius73 Jul 15, 2022

Choose a reason for hiding this comment

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

We can avoid .split(' ')

https://developer.mozilla.org/pt-BR/docs/Web/JavaScript/Reference/Global_Objects/String/includes

Copy link
Contributor Author

@susnux susnux Jul 15, 2022

Choose a reason for hiding this comment

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

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.

}

/**
* @param {Array} tokens - all the tokens in the doc
* @param {number} index - index into the tokens array where to split
Expand Down
3 changes: 3 additions & 0 deletions src/nodes/BulletList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ import { listInputRule } from '../commands/index.js'
* Only there we know the user is not trying to create a task list.
*/
const BulletList = TiptapBulletList.extend({
parseHTML() {
return this.parent().map(rule => Object.assign(rule, { preserveWhitespace: true }))
},

addInputRules() {
return [
Expand Down
2 changes: 1 addition & 1 deletion src/nodes/TaskItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const TaskItem = TipTapTaskItem.extend({
return [
...this.parent(),
wrappingInputRule({
find: /^\s*([-+*])\s(\[(x|X| ?)\])\s$/,
find: /^\s*([-+*])\s(\[(x|X|\s?)\])\s$/,
type: this.type,
getAttributes: match => ({
checked: 'xX'.includes(match[match.length - 1]),
Expand Down
24 changes: 16 additions & 8 deletions src/tests/markdown.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('Markdown though editor', () => {
test('hard breaks', () => {
expect(markdownThroughEditor('hard \nbreak')).toBe('hard \nbreak')
expect(markdownThroughEditor('hard\\\nbreak')).toBe('hard \nbreak')
expect(markdownThroughEditor('no\nbreak')).toBe('no break')
expect(markdownThroughEditor('soft\nbreak')).toBe('soft\nbreak')
})
test('inline format', () => {
expect(markdownThroughEditor('**Test**')).toBe('**Test**')
Expand All @@ -79,7 +79,8 @@ describe('Markdown though editor', () => {
expect(markdownThroughEditor('1. foo\n2. bar')).toBe('1. foo\n2. bar')
})
test('paragraph', () => {
expect(markdownThroughEditor('foo\nbar\n\nfoobar\n\tfoobar')).toBe('foo bar\n\nfoobar foobar')
// Test whitespace characters are untouched
expect(markdownThroughEditor('foo\nbar\n\nfoobar\nfoo\tbar')).toBe('foo\nbar\n\nfoobar\nfoo\tbar')
})
test('links', () => {
expect(markdownThroughEditor('[test](foo)')).toBe('[test](foo)')
Expand All @@ -95,10 +96,15 @@ describe('Markdown though editor', () => {
expect(markdownThroughEditor('```\n<?php echo "Hello World";\n```')).toBe('```\n<?php echo "Hello World";\n```')
})
test('checkboxes', () => {
expect(markdownThroughEditor('- [ ] [asd](sdf)')).toBe('* [ ] [asd](sdf)')
expect(markdownThroughEditor('- [x] [asd](sdf)')).toBe('* [x] [asd](sdf)')
// Invalid ones but should be syntactical unchanged
expect(markdownThroughEditor('- [F] asd')).toBe('* [F] asd')
expect(markdownThroughEditor('- [ [asd](sdf)')).toBe('* [ [asd](sdf)')
// Valid, whitespace is allowed inside the checkbox
expect(markdownThroughEditor('- [\t] asd')).toBe('* [ ] asd')
expect(markdownThroughEditor('- [ ] asd')).toBe('* [ ] asd')
// Valid ones
expect(markdownThroughEditor('- [ ] [asd](sdf)')).toBe('* [ ] [asd](sdf)')
expect(markdownThroughEditor('- [x] [asd](sdf)')).toBe('* [x] [asd](sdf)')
expect(markdownThroughEditor('- [ ] foo\n- [x] bar')).toBe('* [ ] foo\n* [x] bar')
expect(markdownThroughEditor('- [x] foo\n' +
' - [ ] bar\n' +
Expand All @@ -108,10 +114,12 @@ describe('Markdown though editor', () => {
' * [x] baz\n' +
'* [ ] bim')
expect(markdownThroughEditor('- [X] asd')).toBe('* [x] asd')
expect(markdownThroughEditor('- [\t] asd')).toBe('* [ ] asd')
expect(markdownThroughEditor('- [ ] asd')).toBe('* [ ] asd')
expect(markdownThroughEditor('- [X] asd')).toBe('* [x] asd')
expect(markdownThroughEditor('- [F] asd')).toBe('* [F] asd')

expect(markdownThroughEditorHtml('<ul class="contains-task-list"><li><input type="checkbox" checked /><label>foo</label></li></ul>')).toBe('* [x] foo')
expect(markdownThroughEditorHtml('<ul class="contains-task-list"><li><input type="checkbox" /><label>test</label></li></ul>')).toBe('* [ ] test')
expect(markdownThroughEditorHtml('<ul class="contains-task-list"><li><input type="checkbox" checked /><div><h2>Test</h2><p><strong>content</strong></p></div></li></ul>')).toBe('* [x] Test\n\n **content**')
expect(markdownThroughEditorHtml('<ul class="contains-task-list"><li><input type="checkbox" checked /><p>Test</p><h1>Block level headline</h1></li></ul>')).toBe('* [x] Test\n\n # Block level headline')
})

test('horizontal rule', () => {
Expand Down Expand Up @@ -144,7 +152,7 @@ describe('Markdown serializer from html', () => {
test('hard line breaks', () => {
expect(markdownThroughEditorHtml('<p>hard<br />break</p>')).toBe('hard \nbreak')
expect(markdownThroughEditorHtml('<p>hard<br>break</p>')).toBe('hard \nbreak')
expect(markdownThroughEditorHtml('<p>no\nbreak</p>')).toBe('no break')
expect(markdownThroughEditorHtml('<p>soft\nbreak</p>')).toBe('soft\nbreak')
})
test('links', () => {
expect(markdownThroughEditorHtml('<a href="foo">test</a>')).toBe('[test](foo)')
Expand Down
4 changes: 2 additions & 2 deletions src/tests/markdownit.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('markdownit', () => {
const rendered = markdownit.render('* [ ] task\n* not a task')
expect(stripIndent(rendered)).toBe(stripIndent(`
<ul class="contains-task-list">
<li class="task-list-item"><input class="task-list-item-checkbox" disabled="" type="checkbox"> task</li>
<li class="task-list-item "><input class="task-list-item-checkbox" type="checkbox" disabled="" id="task-item-0" />task</li>
</ul>
<ul>
<li>not a task</li>
Expand All @@ -24,7 +24,7 @@ describe('markdownit', () => {
<li>not a task</li>
</ul>
<ul class="contains-task-list">
<li class="task-list-item"><input class="task-list-item-checkbox" disabled="" type="checkbox"> task</li>
<li class="task-list-item "><input class="task-list-item-checkbox" type="checkbox" disabled="" id="task-item-1" />task</li>
</ul>
`))
})
Expand Down