Skip to content
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.

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

BlockAutoformatEditing and InlineAutoformatEditing does not have docs for pluginName property #7290

Closed
tomalec opened this issue May 25, 2020 · 4 comments
Labels
resolution:duplicate This issue is a duplicate of another issue and was merged into it. type:docs This issue reports a task related to documentation (e.g. an idea for a guide).

Comments

@tomalec
Copy link
Contributor

tomalec commented May 25, 2020

📝 Provide a description of requested docs changes

As you can see below pluginName property does not have any docs for BlockAutoformatEditing and InlineAutoformatEditing

These properties were added at 3879b03 with @inheritDoc, but the thing is both classes do not inherit from anything.
https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-autoformat/src/blockautoformatediting.js#L23

Personally, I don't get the design pattern there. They only have a constructor() defined, which does a lot, and the instances returned from constructors are not used anywhere.

Given the instances are not stored anywhere, I'm not sure why pluginName is even needed?

@tomalec tomalec added type:docs This issue reports a task related to documentation (e.g. an idea for a guide). squad:red labels May 25, 2020
@Reinmar
Copy link
Member

Reinmar commented May 26, 2020

Those properties can be removed as these are not plugins.

@jodator
Copy link
Contributor

jodator commented May 26, 2020

My 2 cents: Yep it looks like those can be simple functions - we use that pattern everywhere else.

Personally I don't like creating objects that are not referenced anywhere so it would be nice to refactor that (if we have time, etc).

@tomalec
Copy link
Contributor Author

tomalec commented May 26, 2020

If we're about to do small refactoring, I'd add simplifying arguments handling to the list.

Currently, there is a logic to support, various types of arguments like: testRegexpOrCallback, attributeOrCallback, even though, the only case we actually use is: testRegexp, callback

@Reinmar
Copy link
Member

Reinmar commented May 26, 2020

Let's cover that in #1239.

@Reinmar Reinmar closed this as completed May 26, 2020
@Reinmar Reinmar added the resolution:duplicate This issue is a duplicate of another issue and was merged into it. label May 26, 2020
tomalec added a commit that referenced this issue May 26, 2020
tomalec added a commit that referenced this issue May 30, 2020
…toformatEditing's formatCallback.

It was used only in tests, and was not checking whether the command `.isEnabled`.
Autoformat uses its own `getCallbackFunctionForInlineAutoformat`.
The change was proposed at #7290 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:duplicate This issue is a duplicate of another issue and was merged into it. type:docs This issue reports a task related to documentation (e.g. an idea for a guide).
Projects
None yet
Development

No branches or pull requests

3 participants