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

1640: Remove conversion.register() add conversion.addAlias() #1665

Merged
merged 12 commits into from
Feb 8, 2019
Merged

Conversation

jodator
Copy link
Contributor

@jodator jodator commented Feb 7, 2019

Suggested merge commit message (convention)

Other: Change Conversion class API. Closes ckeditor/ckeditor5#4467.

BREAKING CHANGE: The Conversion#register() method was removed from public API. Use constructor parameters to pass dispatchers and Conversion#addAlias() to register an alternative conversion group for registered upcast or downcast dispatcher.


Additional information

@jodator jodator requested a review from pjasiun February 7, 2019 14:23
@coveralls
Copy link

coveralls commented Feb 7, 2019

Coverage Status

Coverage decreased (-8.4%) to 91.578% when pulling c02a29f on 1640 into c31bea6 on master.

@jodator jodator changed the title 1640: Remove conversion.register() add conversion.addAlias() trollface: 1640: Remove conversion.register() add conversion.addAlias() :trollface: Feb 7, 2019
@jodator jodator changed the title 1640: Remove conversion.register() add conversion.addAlias() :trollface: 1640: Remove conversion.register() add conversion.addAlias() Feb 7, 2019
register( name, conversionHelpers ) {
if ( this._conversionHelpers.has( name ) ) {
addAlias( alias, dispatcher ) {
const groupEntry = [ ...this._groups.entries() ].find( ( [ , dispatchers ] ) => dispatchers.includes( dispatcher ) );
Copy link

Choose a reason for hiding this comment

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

It needs to be simplified :)


const ConversionHelper = this._helpers.get( groupName );

return new ConversionHelper( this._groups.get( groupName ) );
Copy link

Choose a reason for hiding this comment

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

Is it efficient to create a new instance whenever the converter is used? Maybe we could store instances of helpers?

@jodator
Copy link
Contributor Author

jodator commented Feb 8, 2019

Since we only have upcast and downcast types of dispatchers - which have to be present anyway - I've changed the way how the conversion helpers and dispatchers are stored.

So the both groups are stored in separate private fields (easier & faster to find if dispatcher was registered in one of group - simpler code).

The dispatcher groups/aliases are stored in _helpers map as they are returned from conversion.for() method.

@pjasiun pjasiun merged commit e7d09cd into master Feb 8, 2019
@pjasiun pjasiun deleted the 1640 branch February 8, 2019 13:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants