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
Development

Successfully merging this pull request may close these issues.

Remove Conversion#register()
3 participants