From 6eea5a0254a2e69f1ed79522bdab0dead229b998 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 8 Apr 2021 17:39:23 +0200 Subject: [PATCH 1/4] Preserve markers on upcast. --- .../src/codeblockediting.js | 4 +- .../ckeditor5-code-block/src/converters.js | 77 +++++++++++++++---- packages/ckeditor5-code-block/src/utils.js | 37 --------- .../tests/codeblockediting.js | 67 ++++++++++++++++ 4 files changed, 132 insertions(+), 53 deletions(-) diff --git a/packages/ckeditor5-code-block/src/codeblockediting.js b/packages/ckeditor5-code-block/src/codeblockediting.js index 4e58de0d671..41dfe9914ce 100644 --- a/packages/ckeditor5-code-block/src/codeblockediting.js +++ b/packages/ckeditor5-code-block/src/codeblockediting.js @@ -22,7 +22,8 @@ import { import { modelToViewCodeBlockInsertion, modelToDataViewSoftBreakInsertion, - dataViewToModelCodeBlockInsertion + dataViewToModelCodeBlockInsertion, + dataViewToModelTextNewlinesInsertion } from './converters'; const DEFAULT_ELEMENT = 'paragraph'; @@ -131,6 +132,7 @@ export default class CodeBlockEditing extends Plugin { editor.data.downcastDispatcher.on( 'insert:codeBlock', modelToViewCodeBlockInsertion( model, normalizedLanguagesDefs ) ); editor.data.downcastDispatcher.on( 'insert:softBreak', modelToDataViewSoftBreakInsertion( model ), { priority: 'high' } ); editor.data.upcastDispatcher.on( 'element:pre', dataViewToModelCodeBlockInsertion( editor.editing.view, normalizedLanguagesDefs ) ); + editor.data.upcastDispatcher.on( 'text', dataViewToModelTextNewlinesInsertion() ); // Intercept the clipboard input (paste) when the selection is anchored in the code block and force the clipboard // data to be pasted as a single plain text. Otherwise, the code lines will split the code block and diff --git a/packages/ckeditor5-code-block/src/converters.js b/packages/ckeditor5-code-block/src/converters.js index 1131ca7fab9..093b4236faa 100644 --- a/packages/ckeditor5-code-block/src/converters.js +++ b/packages/ckeditor5-code-block/src/converters.js @@ -7,10 +7,7 @@ * @module code-block/converters */ -import { - rawSnippetTextToModelDocumentFragment, - getPropertyAssociation -} from './utils'; +import { getPropertyAssociation } from './utils'; /** * A model-to-view (both editing and data) converter for the `codeBlock` element. @@ -120,11 +117,11 @@ export function modelToDataViewSoftBreakInsertion( model ) { * * Sample input: * - *
foo();\nbar();
+ *
foo();bar();
* * Sample output: * - * foo();bar(); + * foo();bar(); * * @param {module:engine/view/view~View} editingView * @param {Array.} languageDefs The normalized language @@ -151,6 +148,11 @@ export function dataViewToModelCodeBlockInsertion( editingView, languageDefs ) { return; } + // In case of nested `
` we don't want to convert to another code block.
+		if ( data.modelCursor.findAncestor( 'codeBlock' ) ) {
+			return;
+		}
+
 		const { consumable, writer } = conversionApi;
 
 		if ( !consumable.test( viewItem, { name: true } ) || !consumable.test( viewChild, { name: true } ) ) {
@@ -183,15 +185,7 @@ export function dataViewToModelCodeBlockInsertion( editingView, languageDefs ) {
 			writer.setAttribute( 'language', defaultLanguageName, codeBlock );
 		}
 
-		// HTML elements are invalid content for ``.
-		// Read only text nodes.
-		const textData = [ ...editingView.createRangeIn( viewChild ) ]
-			.filter( current => current.type === 'text' )
-			.map( ( { item } ) => item.data )
-			.join( '' );
-		const fragment = rawSnippetTextToModelDocumentFragment( writer, textData );
-
-		writer.append( fragment, codeBlock );
+		conversionApi.convertChildren( viewChild, codeBlock );
 
 		// Let's try to insert code block.
 		if ( !conversionApi.safeInsert( codeBlock, data.modelCursor ) ) {
@@ -204,3 +198,56 @@ export function dataViewToModelCodeBlockInsertion( editingView, languageDefs ) {
 		conversionApi.updateConversionResult( codeBlock, data );
 	};
 }
+
+/**
+ * A view-to-model converter for new line characters in `
`.
+ *
+ * Sample input:
+ *
+ *		
foo();\nbar();
+ * + * Sample output: + * + * foo();bar(); + * + * @returns {Function} Returns a conversion callback. + */ +export function dataViewToModelTextNewlinesInsertion() { + return ( evt, data, { consumable, writer } ) => { + let position = data.modelCursor; + + // When node is already converted then do nothing. + if ( !consumable.test( data.viewItem ) ) { + return; + } + + // When not inside `codeBlock` then do nothing. + if ( !position.findAncestor( 'codeBlock' ) ) { + return; + } + + consumable.consume( data.viewItem ); + + const text = data.viewItem.data; + const textLines = text.split( '\n' ).map( data => writer.createText( data ) ); + const lastLine = textLines[ textLines.length - 1 ]; + + for ( const node of textLines ) { + writer.insert( node, position ); + position = position.getShiftedBy( node.offsetSize ); + + if ( node !== lastLine ) { + const softBreak = writer.createElement( 'softBreak' ); + + writer.insert( softBreak, position ); + position = writer.createPositionAfter( softBreak ); + } + } + + data.modelRange = writer.createRange( + data.modelCursor, + position + ); + data.modelCursor = position; + }; +} diff --git a/packages/ckeditor5-code-block/src/utils.js b/packages/ckeditor5-code-block/src/utils.js index 3386a5bce1a..25720b280df 100644 --- a/packages/ckeditor5-code-block/src/utils.js +++ b/packages/ckeditor5-code-block/src/utils.js @@ -99,43 +99,6 @@ export function getLeadingWhiteSpaces( textNode ) { return textNode.data.match( /^(\s*)/ )[ 0 ]; } -/** - * For plain text containing the code (a snippet), it returns a document fragment containing - * model text nodes separated by soft breaks (in place of new line characters "\n"), for instance: - * - * Input: - * - * "foo()\n - * bar()" - * - * Output: - * - * - * "foo()" - * - * "bar()" - * - * - * @param {module:engine/model/writer~Writer} writer - * @param {String} text The raw code text to be converted. - * @returns {module:engine/model/documentfragment~DocumentFragment} - */ -export function rawSnippetTextToModelDocumentFragment( writer, text ) { - const fragment = writer.createDocumentFragment(); - const textLines = text.split( '\n' ).map( data => writer.createText( data ) ); - const lastLine = textLines[ textLines.length - 1 ]; - - for ( const node of textLines ) { - writer.append( node, fragment ); - - if ( node !== lastLine ) { - writer.appendElement( 'softBreak', fragment ); - } - } - - return fragment; -} - /** * For plain text containing the code (a snippet), it returns a document fragment containing * view text nodes separated by `
` elements (in place of new line characters "\n"), for instance: diff --git a/packages/ckeditor5-code-block/tests/codeblockediting.js b/packages/ckeditor5-code-block/tests/codeblockediting.js index 5d78b61873a..ee2c4a1f549 100644 --- a/packages/ckeditor5-code-block/tests/codeblockediting.js +++ b/packages/ckeditor5-code-block/tests/codeblockediting.js @@ -734,6 +734,26 @@ describe( 'CodeBlockEditing', () => { return editor.destroy(); } ); } ); + + it( 'should convert markers inside pre > code', () => { + editor.conversion.for( 'editingDowncast' ).markerToElement( { view: 'group', model: 'group' } ); + + setModelData( model, + '[]Foo' + ); + + model.change( writer => { + const range = model.createRangeIn( model.document.getRoot().getChild( 0 ) ); + + writer.addMarker( 'group', { range, usingOperation: false } ); + } ); + + expect( getViewData( view ) ).to.equal( + '
' +
+					'[]Foo' +
+				'
' + ); + } ); } ); describe( 'data pipeline m -> v conversion ', () => { @@ -866,6 +886,28 @@ describe( 'CodeBlockEditing', () => { return editor.destroy(); } ); } ); + + it( 'should convert markers inside pre > code', () => { + editor.conversion.for( 'downcast' ).markerToData( { model: 'group' } ); + + setModelData( model, + '[]Foo' + ); + + model.change( writer => { + const range = model.createRangeIn( model.document.getRoot().getChild( 0 ) ); + + writer.addMarker( 'group:foo:bar:baz', { range, usingOperation: false } ); + } ); + + expect( editor.getData() ).to.equal( + '
' +
+					'' +
+						'Foo' +
+					'' +
+				'
' + ); + } ); } ); describe( 'data pipeline v -> m conversion ', () => { @@ -948,6 +990,31 @@ describe( 'CodeBlockEditing', () => { expect( getModelData( model ) ).to.equal( '[]

Foo\'s&"bar"

' ); } ); + it( 'should preserve markers inside pre > code', () => { + editor.conversion.for( 'upcast' ).dataToMarker( { view: 'group' } ); + + editor.setData( + '
' +
+					'' +
+						'
' +
+							'' +
+							'Bar' +
+							'' +
+						'
' + + '
' + + '
' + ); + + expect( model.markers.has( 'group:foo:id' ) ).to.be.true; + + const marker = model.markers.get( 'group:foo:id' ); + + expect( marker.getStart().path ).to.deep.equal( [ 0, 0 ] ); + expect( marker.getEnd().path ).to.deep.equal( [ 0, 3 ] ); + + expect( getModelData( model ) ).to.equal( '[]Bar' ); + } ); + it( 'should be overridable', () => { editor.data.upcastDispatcher.on( 'element:pre', ( evt, data, api ) => { const modelItem = api.writer.createElement( 'codeBlock' ); From 6b6b1973d2ffebb9244416a2c54eb27d8dadc0f9 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 9 Apr 2021 20:09:51 +0200 Subject: [PATCH 2/4] Code-block conversion changed to focus on the code element instead of pre element. --- .../src/codeblockediting.js | 4 +- .../ckeditor5-code-block/src/converters.js | 15 ++-- .../tests/codeblockediting.js | 87 ++++++++++++++++++- 3 files changed, 96 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-code-block/src/codeblockediting.js b/packages/ckeditor5-code-block/src/codeblockediting.js index 41dfe9914ce..563cafae081 100644 --- a/packages/ckeditor5-code-block/src/codeblockediting.js +++ b/packages/ckeditor5-code-block/src/codeblockediting.js @@ -86,6 +86,7 @@ export default class CodeBlockEditing extends Plugin { const editor = this.editor; const schema = editor.model.schema; const model = editor.model; + const view = editor.editing.view; const normalizedLanguagesDefs = getNormalizedAndLocalizedLanguageDefinitions( editor ); @@ -131,7 +132,8 @@ export default class CodeBlockEditing extends Plugin { editor.editing.downcastDispatcher.on( 'insert:codeBlock', modelToViewCodeBlockInsertion( model, normalizedLanguagesDefs, true ) ); editor.data.downcastDispatcher.on( 'insert:codeBlock', modelToViewCodeBlockInsertion( model, normalizedLanguagesDefs ) ); editor.data.downcastDispatcher.on( 'insert:softBreak', modelToDataViewSoftBreakInsertion( model ), { priority: 'high' } ); - editor.data.upcastDispatcher.on( 'element:pre', dataViewToModelCodeBlockInsertion( editor.editing.view, normalizedLanguagesDefs ) ); + + editor.data.upcastDispatcher.on( 'element:code', dataViewToModelCodeBlockInsertion( view, normalizedLanguagesDefs ) ); editor.data.upcastDispatcher.on( 'text', dataViewToModelTextNewlinesInsertion() ); // Intercept the clipboard input (paste) when the selection is anchored in the code block and force the clipboard diff --git a/packages/ckeditor5-code-block/src/converters.js b/packages/ckeditor5-code-block/src/converters.js index 093b4236faa..18b97c03be6 100644 --- a/packages/ckeditor5-code-block/src/converters.js +++ b/packages/ckeditor5-code-block/src/converters.js @@ -141,10 +141,10 @@ export function dataViewToModelCodeBlockInsertion( editingView, languageDefs ) { const defaultLanguageName = languageDefs[ 0 ].language; return ( evt, data, conversionApi ) => { - const viewItem = data.viewItem; - const viewChild = viewItem.getChild( 0 ); + const viewCodeElement = data.viewItem; + const viewPreElement = viewCodeElement.parent; - if ( !viewChild || !viewChild.is( 'element', 'code' ) ) { + if ( !viewPreElement || !viewPreElement.is( 'element', 'pre' ) ) { return; } @@ -155,12 +155,12 @@ export function dataViewToModelCodeBlockInsertion( editingView, languageDefs ) { const { consumable, writer } = conversionApi; - if ( !consumable.test( viewItem, { name: true } ) || !consumable.test( viewChild, { name: true } ) ) { + if ( !consumable.test( viewCodeElement, { name: true } ) ) { return; } const codeBlock = writer.createElement( 'codeBlock' ); - const viewChildClasses = [ ...viewChild.getClassNames() ]; + const viewChildClasses = [ ...viewCodeElement.getClassNames() ]; // As we're to associate each class with a model language, a lack of class (empty class) can be // also associated with a language if the language definition was configured so. Pushing an empty @@ -185,15 +185,14 @@ export function dataViewToModelCodeBlockInsertion( editingView, languageDefs ) { writer.setAttribute( 'language', defaultLanguageName, codeBlock ); } - conversionApi.convertChildren( viewChild, codeBlock ); + conversionApi.convertChildren( viewCodeElement, codeBlock ); // Let's try to insert code block. if ( !conversionApi.safeInsert( codeBlock, data.modelCursor ) ) { return; } - consumable.consume( viewItem, { name: true } ); - consumable.consume( viewChild, { name: true } ); + consumable.consume( viewCodeElement, { name: true } ); conversionApi.updateConversionResult( codeBlock, data ); }; diff --git a/packages/ckeditor5-code-block/tests/codeblockediting.js b/packages/ckeditor5-code-block/tests/codeblockediting.js index ee2c4a1f549..be750081908 100644 --- a/packages/ckeditor5-code-block/tests/codeblockediting.js +++ b/packages/ckeditor5-code-block/tests/codeblockediting.js @@ -12,6 +12,7 @@ import OutdentCodeBlockCommand from '../src/outdentcodeblockcommand'; import AlignmentEditing from '@ckeditor/ckeditor5-alignment/src/alignmentediting'; import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; +import CodeEditing from '@ckeditor/ckeditor5-basic-styles/src/code/codeediting'; import Enter from '@ckeditor/ckeditor5-enter/src/enter'; import ShiftEnter from '@ckeditor/ckeditor5-enter/src/shiftenter'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; @@ -908,6 +909,28 @@ describe( 'CodeBlockEditing', () => { '
' ); } ); + + it( 'should convert markers on a code block', () => { + editor.conversion.for( 'downcast' ).markerToData( { model: 'group' } ); + + setModelData( model, + '[]Foo' + ); + + model.change( writer => { + const range = model.createRangeOn( model.document.getRoot().getChild( 0 ) ); + + writer.addMarker( 'group:foo:bar:baz', { range, usingOperation: false } ); + } ); + + expect( editor.getData() ).to.equal( + '
' +
+					'' +
+						'Foo' +
+					'' +
+				'
' + ); + } ); } ); describe( 'data pipeline v -> m conversion ', () => { @@ -1015,7 +1038,28 @@ describe( 'CodeBlockEditing', () => { expect( getModelData( model ) ).to.equal( '[]Bar' ); } ); - it( 'should be overridable', () => { + it( 'should preserve markers inside pre > code', () => { + editor.conversion.for( 'upcast' ).dataToMarker( { view: 'group' } ); + + editor.setData( + '
' +
+					'' +
+						'Foo' +
+					'' +
+				'
' + ); + + expect( model.markers.has( 'group:foo:id' ) ).to.be.true; + + const marker = model.markers.get( 'group:foo:id' ); + + expect( marker.getStart().path ).to.deep.equal( [ 0 ] ); + expect( marker.getEnd().path ).to.deep.equal( [ 1 ] ); + + expect( getModelData( model ) ).to.equal( '[]Foo' ); + } ); + + it( 'should be overridable (pre)', () => { editor.data.upcastDispatcher.on( 'element:pre', ( evt, data, api ) => { const modelItem = api.writer.createElement( 'codeBlock' ); @@ -1032,6 +1076,27 @@ describe( 'CodeBlockEditing', () => { expect( getModelData( model ) ).to.equal( '[]Hello World!' ); } ); + it( 'should be overridable (code)', () => { + editor.data.upcastDispatcher.on( 'element:code', ( evt, data, api ) => { + if ( !data.viewItem.parent.is( 'element', 'pre' ) ) { + return; + } + + const modelItem = api.writer.createElement( 'codeBlock' ); + + api.writer.appendText( 'Hello World!', modelItem ); + api.writer.insert( modelItem, data.modelCursor ); + api.consumable.consume( data.viewItem, { name: true } ); + + data.modelCursor = api.writer.createPositionAfter( modelItem ); + data.modelRange = api.writer.createRangeOn( modelItem ); + }, { priority: 'high' } ); + + editor.setData( '
Foo Bar
' ); + + expect( getModelData( model ) ).to.equal( '[]Hello World!' ); + } ); + it( 'should split parents to correctly upcast the code block', () => { editor.setData( '

foo

x
bar

' ); @@ -1074,6 +1139,26 @@ describe( 'CodeBlockEditing', () => { expect( getModelData( model, { rootName: 'title', withoutSelection: true } ) ).to.equal( '' ); } ); + it( 'should not conflict with code attribute conversion', async () => { + const element = document.createElement( 'div' ); + document.body.appendChild( element ); + + const editor = await ClassicTestEditor.create( element, { + plugins: [ CodeEditing, CodeBlockEditing, Paragraph ] + } ); + + editor.setData( '
foobar
' ); + + expect( getModelData( editor.model ) ).to.equal( '[]foobar' ); + + editor.setData( 'foobar' ); + + expect( getModelData( editor.model ) ).to.equal( '<$text code="true">[]foobar' ); + + await editor.destroy(); + element.remove(); + } ); + describe( 'config.codeBlock.languages', () => { it( 'should be respected when upcasting', () => { return ClassicTestEditor.create( From 40788e7356fed2d8f892c309572555f2d44bd78e Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 13 Apr 2021 09:24:13 +0200 Subject: [PATCH 3/4] Update packages/ckeditor5-code-block/src/converters.js --- packages/ckeditor5-code-block/src/converters.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-code-block/src/converters.js b/packages/ckeditor5-code-block/src/converters.js index 18b97c03be6..227847a0365 100644 --- a/packages/ckeditor5-code-block/src/converters.js +++ b/packages/ckeditor5-code-block/src/converters.js @@ -148,7 +148,7 @@ export function dataViewToModelCodeBlockInsertion( editingView, languageDefs ) { return; } - // In case of nested `
` we don't want to convert to another code block.
+		// In case of nested code blocks we don't want to convert to another code block.
 		if ( data.modelCursor.findAncestor( 'codeBlock' ) ) {
 			return;
 		}

From df84fe36a26c2c0910d1f538aa52d4e315a4251e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?=
 
Date: Tue, 13 Apr 2021 09:59:32 +0200
Subject: [PATCH 4/4] Update
 packages/ckeditor5-code-block/tests/codeblockediting.js

Co-authored-by: Szymon Cofalik 
---
 packages/ckeditor5-code-block/tests/codeblockediting.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/ckeditor5-code-block/tests/codeblockediting.js b/packages/ckeditor5-code-block/tests/codeblockediting.js
index be750081908..c80a93250f8 100644
--- a/packages/ckeditor5-code-block/tests/codeblockediting.js
+++ b/packages/ckeditor5-code-block/tests/codeblockediting.js
@@ -1038,7 +1038,7 @@ describe( 'CodeBlockEditing', () => {
 			expect( getModelData( model ) ).to.equal( '[]Bar' );
 		} );
 
-		it( 'should preserve markers inside pre > code', () => {
+		it( 'should preserve markers on a code block', () => {
 			editor.conversion.for( 'upcast' ).dataToMarker( { view: 'group' } );
 
 			editor.setData(