Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Removed usage of logger. #1760

Merged
merged 8 commits into from
Jul 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 12 additions & 11 deletions src/conversion/downcasthelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/**
* Contains downcast (model-to-view) converters for {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher}.
*
* @module engine/conversion/downcasthelpers
*/

import ModelRange from '../model/range';
import ModelSelection from '../model/selection';
import ModelElement from '../model/element';
Expand All @@ -11,14 +17,8 @@ import ViewAttributeElement from '../view/attributeelement';
import DocumentSelection from '../model/documentselection';
import ConversionHelpers from './conversionhelpers';

import log from '@ckeditor/ckeditor5-utils/src/log';
import { cloneDeep } from 'lodash-es';

/**
* Contains downcast (model-to-view) converters for {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher}.
*
* @module engine/conversion/downcasthelpers
*/
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

/**
* Downcast conversion helper functions.
Expand Down Expand Up @@ -823,10 +823,11 @@ function changeAttribute( attributeCreator ) {
*
* @error conversion-attribute-to-attribute-on-text
*/
log.warn( 'conversion-attribute-to-attribute-on-text: ' +
'Trying to convert text node\'s attribute with attribute-to-attribute converter.' );

return;
throw new CKEditorError(
'conversion-attribute-to-attribute-on-text: ' +
'Trying to convert text node\'s attribute with attribute-to-attribute converter.',
[ data, conversionApi ]
);
}

// First remove the old attribute if there was one.
Expand Down
8 changes: 1 addition & 7 deletions src/model/documentselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import TextProxy from './textproxy';
import toMap from '@ckeditor/ckeditor5-utils/src/tomap';
import Collection from '@ckeditor/ckeditor5-utils/src/collection';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import log from '@ckeditor/ckeditor5-utils/src/log';
import uid from '@ckeditor/ckeditor5-utils/src/uid';

const storePrefix = 'selection:';
Expand Down Expand Up @@ -804,12 +803,7 @@ class LiveSelection extends Selection {
this._checkRange( range );

if ( range.root == this._document.graveyard ) {
/**
* Trying to add a Range that is in the graveyard root. Range rejected.
*
* @warning model-selection-range-in-graveyard
*/
log.warn( 'model-selection-range-in-graveyard: Trying to add a Range that is in the graveyard root. Range rejected.' );
// @if CK_DEBUG // console.warn( 'Trying to add a Range that is in the graveyard root. Range rejected.' );

return;
}
Expand Down
17 changes: 8 additions & 9 deletions src/model/operation/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import Range from '../range';
import Position from '../position';

import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
import log from '@ckeditor/ckeditor5-utils/src/log';

const transformations = new Map();

Expand Down Expand Up @@ -102,14 +101,14 @@ export function transform( a, b, context = {} ) {

return transformationFunction( a, b, context );
} catch ( e ) {
log.error( 'Error during operation transformation!', e.message );
log.error( 'Transformed operation', a );
log.error( 'Operation transformed by', b );
log.error( 'context.aIsStrong', context.aIsStrong );
log.error( 'context.aWasUndone', context.aWasUndone );
log.error( 'context.bWasUndone', context.bWasUndone );
log.error( 'context.abRelation', context.abRelation );
log.error( 'context.baRelation', context.baRelation );
// @if CK_DEBUG // console.error( 'Error during operation transformation!', e.message );
// @if CK_DEBUG // console.error( 'Transformed operation', a );
// @if CK_DEBUG // console.error( 'Operation transformed by', b );
// @if CK_DEBUG // console.error( 'context.aIsStrong', context.aIsStrong );
// @if CK_DEBUG // console.error( 'context.aWasUndone', context.aWasUndone );
// @if CK_DEBUG // console.error( 'context.bWasUndone', context.bWasUndone );
// @if CK_DEBUG // console.error( 'context.abRelation', context.abRelation );
// @if CK_DEBUG // console.error( 'context.baRelation', context.baRelation );

throw e;
}
Expand Down
34 changes: 21 additions & 13 deletions src/model/utils/insertcontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import Position from '../position';
import LivePosition from '../liveposition';
import Element from '../element';
import Range from '../range';
import log from '@ckeditor/ckeditor5-utils/src/log';
import DocumentSelection from '../documentselection';
import Selection from '../selection';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

/**
* Inserts content into the editor (specified selection) as one would expect the paste
Expand Down Expand Up @@ -85,13 +85,8 @@ export default function insertContent( model, content, selectable, placeOrOffset
} else {
// We are not testing else because it's a safe check for unpredictable edge cases:
// an insertion without proper range to select.

/**
* Cannot determine a proper selection range after insertion.
*
* @warning insertcontent-no-range
*/
log.warn( 'insertcontent-no-range: Cannot determine a proper selection range after insertion.' );
//
// @if CK_DEBUG // console.warn( 'Cannot determine a proper selection range after insertion.' );
}

const affectedRange = insertion.getAffectedRange() || model.createRange( insertionPosition );
Expand Down Expand Up @@ -322,12 +317,19 @@ class Insertion {
if ( !this.schema.checkChild( this.position, node ) ) {
// Algorithm's correctness check. We should never end up here but it's good to know that we did.
// Note that it would often be a silent issue if we insert node in a place where it's not allowed.
log.error(
'insertcontent-wrong-position: The node cannot be inserted on the given position.',

/**
* Given node cannot be inserted on the given position.
*
* @error insertcontent-wrong-position
* @param {module:engine/model/node~Node} node Node to insert.
* @param {module:engine/model/position~Position} position Position to insert the node at.
*/
throw new CKEditorError(
'insertcontent-wrong-position: Given node cannot be inserted on the given position.',
this,
{ node, position: this.position }
);

return;
}

const livePos = LivePosition.fromPosition( this.position, 'toNext' );
Expand Down Expand Up @@ -442,7 +444,13 @@ class Insertion {
// Algorithm's correctness check. We should never end up here but it's good to know that we did.
// At this point the insertion position should be after the node we'll merge. If it isn't,
// it should need to be secured as in the left merge case.
log.error( 'insertcontent-wrong-position-on-merge: The insertion position should equal the merge position' );
/**
* An internal error occured during merging insertion content with siblings.
* The insertion position should equal to the merge position.
*
* @error insertcontent-invalid-insertion-position
*/
throw new CKEditorError( 'insertcontent-invalid-insertion-position', this );
}

// Move the position to the previous node, so it isn't moved to the graveyard on merge.
Expand Down
16 changes: 6 additions & 10 deletions src/view/observer/selectionobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

import Observer from './observer';
import MutationObserver from './mutationobserver';
import log from '@ckeditor/ckeditor5-utils/src/log';
import { debounce } from 'lodash-es';

/**
Expand Down Expand Up @@ -150,15 +149,12 @@ export default class SelectionObserver extends Observer {
// This counter is reset each second. 60 selection changes in 1 second is enough high number
// to be very difficult (impossible) to achieve using just keyboard keys (during normal editor use).
if ( ++this._loopbackCounter > 60 ) {
/**
* Selection change observer detected an infinite rendering loop.
* Most probably you try to put the selection in the position which is not allowed
* by the browser and browser fixes it automatically what causes `selectionchange` event on
* which a loopback through a model tries to re-render the wrong selection and again.
*
* @error selectionchange-infinite-loop
*/
log.warn( 'selectionchange-infinite-loop: Selection change observer detected an infinite rendering loop.' );
// Selection change observer detected an infinite rendering loop.
// Most probably you try to put the selection in the position which is not allowed
// by the browser and browser fixes it automatically what causes `selectionchange` event on
// which a loopback through a model tries to re-render the wrong selection and again.
//
// @if CK_DEBUG // console.warn( 'Selection change observer detected an infinite rendering loop.' );

return;
}
Expand Down
14 changes: 5 additions & 9 deletions src/view/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import CompositionObserver from './observer/compositionobserver';
import InputObserver from './observer/inputobserver';

import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
import log from '@ckeditor/ckeditor5-utils/src/log';
import mix from '@ckeditor/ckeditor5-utils/src/mix';
import { scrollViewportToShowTarget } from '@ckeditor/ckeditor5-utils/src/dom/scroll';
import { injectUiElementHandling } from './uielement';
Expand Down Expand Up @@ -398,14 +397,11 @@ export default class View {
this.domConverter.focus( editable );
this.forceRender();
} else {
/**
* Before focusing view document, selection should be placed inside one of the view's editables.
* Normally its selection will be converted from model document (which have default selection), but
* when using view document on its own, we need to manually place selection before focusing it.
*
* @error view-focus-no-selection
*/
log.warn( 'view-focus-no-selection: There is no selection in any editable to focus.' );
// Before focusing view document, selection should be placed inside one of the view's editables.
// Normally its selection will be converted from model document (which have default selection), but
// when using view document on its own, we need to manually place selection before focusing it.
//
// @if CK_DEBUG // console.warn( 'There is no selection in any editable to focus.' );
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/conversion/conversion.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe( 'Conversion', () => {

it( 'should be chainable', () => {
const helpers = conversion.for( 'upcast' );
const addResult = helpers.add( () => { } );
const addResult = helpers.add( () => {} );

expect( addResult ).to.equal( helpers );
} );
Expand Down
4 changes: 2 additions & 2 deletions tests/conversion/downcastdispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,8 @@ describe( 'DowncastDispatcher', () => {
const viewFigure = new ViewContainerElement( 'figure', null, viewCaption );

// Create custom highlight handler mock.
viewFigure._setCustomProperty( 'addHighlight', () => { } );
viewFigure._setCustomProperty( 'removeHighlight', () => { } );
viewFigure._setCustomProperty( 'addHighlight', () => {} );
viewFigure._setCustomProperty( 'removeHighlight', () => {} );

// Create mapper mock.
dispatcher.conversionApi.mapper = {
Expand Down
34 changes: 13 additions & 21 deletions tests/conversion/downcasthelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

/* globals console */

import EditingController from '../../src/controller/editingcontroller';

import Model from '../../src/model/model';
Expand All @@ -15,7 +17,6 @@ import ViewContainerElement from '../../src/view/containerelement';
import ViewUIElement from '../../src/view/uielement';
import ViewText from '../../src/view/text';

import log from '@ckeditor/ckeditor5-utils/src/log';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';

import DowncastHelpers, {
Expand All @@ -32,6 +33,7 @@ import { stringify as stringifyView } from '../../src/dev-utils/view';
import View from '../../src/view/view';
import createViewRoot from '../view/_utils/createroot';
import { setData as setModelData } from '../../src/dev-utils/model';
import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';

describe( 'DowncastHelpers', () => {
let model, modelRoot, viewRoot, downcastHelpers, controller;
Expand Down Expand Up @@ -614,7 +616,7 @@ describe( 'DowncastHelpers', () => {

// #1587
it( 'config.view and config.model as strings in generic conversion (elements only)', () => {
const logStub = testUtils.sinon.stub( log, 'warn' );
const consoleWarnStub = testUtils.sinon.stub( console, 'warn' );

downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );

Expand All @@ -626,7 +628,7 @@ describe( 'DowncastHelpers', () => {
} );

expectResult( '<p test="1"></p><p test="2"></p>' );
expect( logStub.callCount ).to.equal( 0 );
expect( consoleWarnStub.callCount ).to.equal( 0 );

model.change( writer => {
writer.removeAttribute( 'test', modelRoot.getChild( 1 ) );
Expand All @@ -637,29 +639,19 @@ describe( 'DowncastHelpers', () => {

// #1587
it( 'config.view and config.model as strings in generic conversion (elements + text)', () => {
const logStub = testUtils.sinon.stub( log, 'warn' );

downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );

downcastHelpers.attributeToAttribute( { model: 'test', view: 'test' } );

model.change( writer => {
writer.insertElement( 'paragraph', modelRoot, 0 );
writer.insertElement( 'paragraph', { test: '1' }, modelRoot, 1 );

writer.insertText( 'Foo', { test: '2' }, modelRoot.getChild( 0 ), 0 );
writer.insertText( 'Bar', { test: '3' }, modelRoot.getChild( 1 ), 0 );
} );

expectResult( '<p>Foo</p><p test="1">Bar</p>' );
expect( logStub.callCount ).to.equal( 2 );
expect( logStub.alwaysCalledWithMatch( 'conversion-attribute-to-attribute-on-text' ) ).to.true;

model.change( writer => {
writer.removeAttribute( 'test', modelRoot.getChild( 1 ) );
} );
expectToThrowCKEditorError( () => {
model.change( writer => {
writer.insertElement( 'paragraph', modelRoot, 0 );
writer.insertElement( 'paragraph', { test: '1' }, modelRoot, 1 );

expectResult( '<p>Foo</p><p>Bar</p>' );
writer.insertText( 'Foo', { test: '2' }, modelRoot.getChild( 0 ), 0 );
writer.insertText( 'Bar', { test: '3' }, modelRoot.getChild( 1 ), 0 );
} );
}, /^conversion-attribute-to-attribute-on-text/ );
} );

it( 'should convert attribute insert/change/remove on a model node', () => {
Expand Down
4 changes: 2 additions & 2 deletions tests/model/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe( 'Document', () => {
baseVersion: 0,
isDocumentOperation: true,
_execute: sinon.stub().returns( data ),
_validate: () => { }
_validate: () => {}
};

batch = new Batch();
Expand Down Expand Up @@ -82,7 +82,7 @@ describe( 'Document', () => {
const operation = {
baseVersion: 1,
isDocumentOperation: true,
_execute: () => { }
_execute: () => {}
};

expectToThrowCKEditorError( () => {
Expand Down
Loading