Opened 6 years ago

Closed 6 years ago

#10013 closed Bug (fixed)

CKBuilder: detect plugin requirements in a smarter way

Reported by: Wiktor Walc Owned by:
Priority: Normal Milestone:
Component: Project : CKBuilder Version: 4.0
Keywords: Cc:

Description

#9829 introduced a requires property.

The horizontalrrule command specified there that it requires a HTML tag:

requires: 'hr'

unfortunately CKBuilder thought that the plugin requires a hr plugin, which is of course wrong.

Suggestion from Piotrek: requires (plugin requirement) should be only searched in the first line of a plugin definition.

It might be actually a reasonable approach: try to find requires as long as next line contains things like: lang, icons properties, empty lines or lines with comments.

Change History (3)

comment:1 Changed 6 years ago by Jakub Ś

Status: newconfirmed
Version: 4.0

comment:2 Changed 6 years ago by Piotrek Koszuliński

To avoid this problem and other conflicts I changed names introduced in #9829. So it's not a problem any more.

However, plugin's developer may still use "requires" property in different cases so CKBuilder should be more precise anyway. As Wiktor mentioned in ticket's desc I'm for requiring that "requires" property is the first property in the plugin's definition. It doesn't have to be in the first line, it'd be enough if it was the first one. Or at least - one of few standard, known properties like "icons", "languages", etc. which are easy to parse.

Of course, this requirement may break some 3rd party plugins, but I think that at some point we may more issues with extracting plugin's metadata if we won't standardise this format.

Also, it's very good practise when important metadata are the first properties in plugin's def. I saw some plugins when they were defined at the end, after entire init method and that was confusing. E.g. I see plugin's def and I don't know what it requires.

Another good pratise would be to always start plugin's file with plugin definition. I find it really irritating when I have to look for plugin's def in 2kLOC plugin file.

comment:3 Changed 6 years ago by Wiktor Walc

Resolution: fixed
Status: confirmedclosed

This has been somehow improved in 1.6.1. Unfortunately there is no reliable way to detect where the plugin definition starts. Checking code straight after CKEDITOR.plugins.add does not work in case of plugins like liststyle, which does the following:

CKEDITOR.plugins.add( 'liststyle', CKEDITOR.plugins.liststyle );

(CKEDITOR.plugins.liststyle is defined earlier in the code)

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy