Renamed and simplified AttributeCommand.

[oskarwrobel]

Fixes ckeditor/ckeditor5#2845

Related to: ckeditor/ckeditor5-basic-styles#25

[oskarwrobel]

I'm not sure if it is the right place for those helpers src/command/helpers?

[oskarwrobel]

My second concern is that extracting helpers to separate files makes them difficult to stubbing. Because of it I duplicated some code in tests instead of making a simple spy:

Helper test:
https://github.com/ckeditor/ckeditor5-core/blob/t/9/tests/command/helpers/isattributeallowedinselection.js#L16-L86

Duplicated code in method which uses helper:
https://github.com/ckeditor/ckeditor5-core/blob/t/9/tests/command/toggleattributecommand.js#L206-L264

[Edit]
After talking with @pjasiun I've decided to keep helpers fully tested, but methods where helpers are used are tested basically, so only few test cases are duplicated.

[oskarwrobel]

I renamed AttributeCommand to ToggleAttributeCommand, but now I'm thinking about AttributeTogglerCommand?

[Reinmar]

And I'm still thinking about BooleanAttributeCommand :D. But from the two you proposed, I'm more for ToggleAttributeCommand. It's more in line with the names that we always used (in terms of grammar).

[oskarwrobel]

I think @pjasiun is also for BooleanAttributeCommand. It's OK for me too, so I can change it.

[pjasiun]

I prefer: ToggleAttributeCommand or even ToggleCommand. The command is an action (AddSomething, RemoveSomething...), it should not be a noun.

[Reinmar]

Yeah, @pjasiun is right. It should be a verb. Though, we used nouns sometimes for features. But perhaps that was wrong – command is an action + state, so its name should indeed represent that and, also, there should be no command like image which insert and edit image at the same time. OTOH, such commands are useful, because they you can use one in your button. Tricky :).

Anyway, ToggleAttributeCommand is fine for me. ToggleCommand isn't, because it doesn't says what it toggles.

[scofalik]

I don't like such long function names and file names but I that's just subjective opinion. Other than that it's ok so I am merging it.

[Reinmar]

I agree that the naming and location isn't the best. Those functions are not commands' helpers. They can be used by any piece of code. So first of all, we need to understand where they belong (I'm thinking e.g. about the schema itself – perhaps static methods). Then, how they actually can be proposed (we need to understand whether they are part of some closed functionality or their number may grow indefinitely).

[Reinmar]

I reported https://github.com/ckeditor/ckeditor5-core/issues/14 for this.