Colon-triggered block formatter and line formatter do not coexist

[jakebailey]

In working on the line formatter (#2690), I discovered the registration of the OnEnterFormater overrides the registration of BlockFormatProviders in extension.ts.

The current code looks like this (after the introduction of the second line in #649):

context.subscriptions.push(languages.registerOnTypeFormattingEditProvider(PYTHON, new BlockFormatProviders(), ':'));
context.subscriptions.push(languages.registerOnTypeFormattingEditProvider(PYTHON, new OnEnterFormatter(), '\n'));

The last registration appears to take precedence, so you can hit enter to format a line, but if you type a :, nothing happens, say after the else in this example:

if x:
    pass
    else

If you swap the registration order so that BlockFormatProviders comes last, line formatting no longer works, but hitting : in the above code gets reformatted into:

if x:
    pass
else:

I think this may be the cause of (or at least fix) #771, since you hit : after else before you would hit enter, meaning that the else would have already been moved left to the correct position.

The documentation for registerOnTypeFormattingEditProvider states that multiple providers can be registered, and will be selected based on a "score". If the score is the same, then the last one wins, which is what is happening here. The "first" trigger character differing between two registrations doesn't seem to matter. The solution is probably to make a single OnTypeFormattingEditProvider which then dispatches based on the trigger character, like:

context.subscriptions.push(languages.registerOnTypeFormattingEditProvider(PYTHON, new NewfangledProvider(), '\n', [':']));

[jakebailey]

I have a working fix in a branch on my fork, but I haven't implemented the tests for the new code yet.

It'd probably also be a good idea to write some tests to ensure that this sort of thing doesn't happen again. @DonJayamanne mentioned to me that this is probably a consequence of only testing the individual parts and not together.

[DonJayamanne]

I have a working fix in a branch on my fork, but I haven't implemented the tests for the new code yet.

Please do publish a PR, or point us to the solution.

It'd probably also be a good idea to write some tests to ensure that this sort of thing doesn't happen

We can write the tests if you aren't planning on doing. If you aren't, then please do create an issue, so we do not forget about it. Thanks