Refactor *AutoformatEditing.js

[tomalec]

This is a followup after #1239 and #7290 (comment)

Provide a description of the task

During the implementation of #7346 I found a few things that look like a code smell to me:

1. inlineAutoformatEditing.js takes testRegexpOrCallback as an argument, while only regExp is used, and the callback is only used in test suite.

   In my opinion, this functionality could be dropped.

2. blockAutoformatEditing behaves inconsistently when it comes to checking if the command is enabled. Usually, it checks it early on data change, but for headings, it checks it after is performed all the matches. Also, inlineAutoformatEditing checks it late.

   In my opinion, it should be checked early for all cases, to avoid pointless computation.

3. Both *AutoformatEditings basically do (pseudocode):

   editor.model.document.on( 'change:data', ( evt, batch ) => {
   	if ( !isEverytighEnabled || !isnotTyping ) {
   		 return false;
   	}
   	// Only difference:
   		text = getDataToCheck();
   
   	match = patternMatch( text );
   	match && editor.model.enqueueChange( writer => {
   		formatCallback()
   	} );
   } );
   

   I believe we can make it DRYier, by letting one extend the other, or moving common code to autoformat.js, and let inlineAutoformatEditing.js, blockAutoformatEditing.js deliver functionality that really differs.

📃 Other details

• Browser: n/a
• OS: n/a
• CKEditor version: https://github.com/ckeditor/ckeditor5/tree/a50e0962a1bd8f2c2ad0b809ecdd963b03397af3
• Installed CKEditor plugins: Autoformat

[tomalec]

GitHub Writer already have those and other problems addressed, see https://github.com/ckeditor/github-writer/blob/7aacc28ca6978b319ddbf85f8387025d3629e821/src/app/plugins/autoformat.js

We could start by porting its structure back to ckeditor5. The behavior is slightly different but more intuitive in my opinion, for sure it's worth considering. I believe we can also, take the structure keeping the current behavior as well.

---

I reached out @fredck for his comments on that. (I hope he will not mind posting them here)

The plugin is pretty complete and I’m very happy with the format of the API. Also, it doesn’t depend on any other code in GHW. It cannot be taken as is ofc, but it is a matter of just cleaning it up with things that don’t make sense for a generic plugin, but I believe there are really very little to be done:

• As you can see, some comments are wrong :D
  https://github.com/ckeditor/github-writer/blob/7aacc28ca6978b319ddbf85f8387025d3629e821/src/app/plugins/autoformat.js#L29
• I’ve enabled all these because I know that these features are available in the editor. You guys may decide whether the feature themselves would enabled them or you would centralize it in this plugin:
  https://github.com/ckeditor/github-writer/blob/7aacc28ca6978b319ddbf85f8387025d3629e821/src/app/plugins/autoformat.js#L51-L81
• In any case, you can safely register all features in this plugin, because in any case the plugin makes a “is the feature available” check and ignore anything that the editor doesn’t support:
  https://github.com/ckeditor/github-writer/blob/7aacc28ca6978b319ddbf85f8387025d3629e821/src/app/plugins/autoformat.js#L51-L81
• You should cleanup the code from my internal debugging stuff:
  https://github.com/ckeditor/github-writer/blob/7aacc28ca6978b319ddbf85f8387025d3629e821/src/app/plugins/autoformat.js#L207-L209

I would be happy to remove the plugin from GHW and use the official one, so let me know if there is a GitHub issue to follow about this.

Guys were talking about exactly this feature yesterday in #github-writer. There is a small annoyance with it as it’s not working when auto-formatting inside the parenthesis, like (bold). I was aware of it and wanted to fix it. I think it is just a matter of fixing these two regexes:

• https://github.com/ckeditor/github-writer/blob/7aacc28ca6978b319ddbf85f8387025d3629e821/src/app/plugins/autoformat.js#L228
• https://github.com/ckeditor/github-writer/blob/7aacc28ca6978b319ddbf85f8387025d3629e821/src/app/plugins/autoformat.js#L252

Finally, you have also 100% test coverage for it here:
https://github.com/ckeditor/github-writer/blob/7aacc28ca6978b319ddbf85f8387025d3629e821/tests/unit/plugins/autoformat.js
GHW tests are compatible with CKEditor 5 tests, so you can more or less copy the file as is.

[jodator]

Related: #2400 - there was an idea to re-use TextWatcher in those plugins. The TextWatcher might need some polishing though.

[Reinmar]

cc @mlewand

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).