Implement Document on Type Formatting

[rchl]

Add support for Document on Type Formatting Request.

For this functionality it's essential that we ignore edits triggered by undo/redo/paste since in that case formatting shouldn't trigger. Of course ST doesn't tell us explicitly how the edit was triggered so I'm listening for those specific commands and annotating the next TextChange with that command name. This should be pretty reliable given that we only consider change to be triggered by that command if it happened "right after" the command (within next tick).

LSP-basedpyright implements this functionality to automatically add f in front a string when typing {. It advertises this capability:

    "documentOnTypeFormattingProvider": {
      "firstTriggerCharacter": "{"
    },

Some thoughts that I had while implementing this:

• When ST auto-inserts matching brace then we don't trigger the request since the last character is not {. Spec mentions that the reported ch should not necessarily be the last character for cases exactly like this but implementing such logic without some help from ST would rely on heuristics so not sure we should do it. Not handling it makes the feature a lot less useful though as ST often does match brace depending on where in the string we insert it.
• Should we apply text edits without version even if document version has changed since the request was triggered (note that LSP-basedpyright returns TextEdits without version)? It should be safe in this particular case of inserting f but might not be safe in general. We currently ignore the edit if document has changed.
• I've tried this feature with the default debouncing we do before notifying didChange and the experience was not good - it took too long to insert an edit (and it was too easy to drop the edit if something was typed in the meantime). So in this change, if we detect we should trigger a request, we purge the changes and trigger request immediately instead of waiting for debounce.

[jwortmann]

• Should we apply text edits without version even if document version has changed since the request be triggered (note that LSP-basedpyright returns TextEdits without version)?

The response is of type TextEdit[], and TextEdit never has a version. So the only thing we can do is to track the version from when the request was sent, and hope that the response arrives and is handled before the next change. It looks like this is what you already do here.

What do you think about making SessionBuffer.purge_changes_async return its version, and use that in the subsequent onTypeFormatting request? That could eliminated a rare case where the document version changed between sending didChange and the onTypeFormatting request (because it runs in on_text_changed_async on the async thread), in which case the server would calculate the edits for the version that is not the current one anymore, but we miss that version being outdated already.

[rchl]

What do you think about making SessionBuffer.purge_changes_async return its version, and use that in the subsequent onTypeFormatting request? That could eliminated a rare case where the document version changed between sending didChange

I've just changed the argument to existing change_count that was in scope which should cover this case.
Not sure it's necessary to change purge_changes_async for this use case.

[rchl]

• When ST auto-inserts matching brace then we don't trigger the request since the last character is not {. Spec mentions that the reported ch should not necessarily be the last character for cases exactly like this but implementing such logic without some help from ST would rely on heuristics so not sure we should do it. Not handling it makes the feature a lot less useful though as ST often does match brace depending on where in the string we insert it.

I feel like this should be implemented actually. The whole point of this feature (at least in case of basedpyright) is kinda lost without it.

VSCode handles it, Zed doesn't.

[jwortmann]

I agree that it would be very useful, and the specs exlicitly mention the automatic brace completion, so I think we should try to support this.

You could also check the penultimate character position via last_str[-2] if last_str is long enough.

Maybe something like

if last_str := changes[-1].str:
    if (ch := last_str[-1]) in self._on_type_formatting_trigger_characters \
            or len(last_str) >= 2 and (ch := last_str[-2]) in self._on_type_formatting_trigger_characters:
        return {
            **text_document_position_params(view, selection.a),
            'options': formatting_options(view.settings()),
            'ch': ch,
        }

I haven't tested whether that works or if there is a way to simplify that more. Ideally we want to keep this _get_on_type_formatting_params_async method as efficient as possible, because unlike most other features it is not debounced and is called on every key press.

[rchl]

I would actually try to make it a bit more specific and less error-prone by only checking second-to-last character if the last two characters match a specific pair ([], (), '', "" etc). But yes, I think it's doable.

[jwortmann]

I would actually try to make it a bit more specific and less error-prone by only checking second-to-last character if the last two characters match a specific pair ([], (), '', "" etc). But yes, I think it's doable.

Yes, sounds like a useful idea.

Another aspect that could be used is that such matched chars are always (I believe) inserted via insert_snippet command by ST. Maybe we could make use of that, since you already listen for some special commands and pass that as the action argument.

[rchl]

Another aspect that could be used is that such matched chars are always (I believe) inserted via insert_snippet command by ST. Maybe we could make use of that, since you already listen for some special commands and pass that as the action argument.

That's true but it might be overkill in this case and even less functional because there can be cases where user just inserts {} manually fast (in places where auto-matching doesn't trigger).

[jwortmann]

I would suggest that we add a global setting "format_on_type": true | false, which should be false by default, because I expect it to be quite controversial if the line where you are typing is automatically formatted, or if there are suddenly things like the f-string added whenever you type a { in a string. So this should probably be opt-in.

I know that we can still always disable features via disabled_capabilities, but this is not very intuitive for users and you either need to have in-depth LSP knowledge to know the correct provider name, or do some research first on our docs website and in the LSP specs, to find out where and how exactly to turn it off. And you would need to disable it for every server config individiually.

---

By the way, similarly I am thinking about adding a new setting "show_signature_help": true | false, which is true by default. Most other editors seems to have a setting for that (e.g. "editor.parameterHints.enabled" in VSCode), and every few months someone on the forum creates a new topic and asks how to disable it. I think it would be good to have settings for all features which probably not everybody wants to use, that easily allow to disable them (we already have that for most things: "show_inlay_hints", "semantic_highlighting", "show_code_actions": "", "show_diagnostics_severity_level": 0, "document_highlight_style": "", etc).

[rchl]

The counterargument for adding a setting is that in case of basedpyright it provides a setting for enabling this functionality and it's not enabled by default. So I would say that it would be more confusing for user to find this basedpyright setting, enable it and see that nothing is happening.

If we're adding a setting then I would be more for having it enabled by default. I believe that it would be better for the user to notice the functionality and look for how to disable it if it's not wanted than to never notice it and potentially miss out on something useful. (That said, it wouldn't apply to basedpyright anyway since its setting is disabled by default).

[jwortmann]

The counterargument for adding a setting is that in case of basedpyright it provides a setting for enabling this functionality and it's not enabled by default. So I would say that it would be more confusing for user to find this basedpyright setting, enable it and see that nothing is happening.

A lot of servers provide settings to enable/disable various features like inlay hints or semantic highlighting on the servers side. So that is already the case for those features, and I think it is reasonable for a new user who freshly installed LSP to go through all settings in the LSP settings file and configure it to their liking. I would take the fact that it's disabled in basedpyright by default also as a hint that this feature can be controversal (personally I think I will not use it, because I don't like the editor interfering too much in what I typed) and should rather be opt-in. Note that we have "lsp_format_on_paste" also disabled by default.

I'm planning a moderate revision of the features page of the docs website, and I could add an info box there about that format-on-type has to be enabled in the user settings.

That said, in case everyone other than me would be in favor of having it enabled by default, I would give in and say having a setting for it that is enabled by default is still better than having no setting at all.

P.S: I was contemplating whether it should be "format_on_type" or rather "lsp_format_on_type" and also be supported in project-specific settings. But I came to the conclusion that imo the regular "format_on_type" is better, because the project-specific settings are most useful for projects with certain formatting or code style rules, typically enforced via some configuration file that is commited to the repository. While this format-on-type feature is more of a small quality-of-life utility provided by the editor, and not really specific to project configurations. In retrospective I would even say "lsp_format_on_paste" should also better have been just a non-project-specific "format_on_paste", because it's more of a user-specific utility rather than project-specific (if the project is configured with formatting rules and you want to enforce that, the "lsp_format_on_save" would cover the formatting for the entire file when saving anyway).

[rchl]

@predragnikolic would you like to add something? :)

[predragnikolic]

Tomorrow in the evening I will try to find some time to have a look at this.

[netlify]

✅ Deploy Preview for sublime-lsp ready!

Name	Link
🔨 Latest commit	998fbb7
🔍 Latest deploy log	https://app.netlify.com/projects/sublime-lsp/deploys/69a36534811f13000811bbea
😎 Deploy Preview	https://deploy-preview-2776--sublime-lsp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[netlify]

✅ Deploy Preview for sublime-lsp ready!

Name	Link
🔨 Latest commit	3c66545
🔍 Latest deploy log	https://app.netlify.com/projects/sublime-lsp/deploys/69a3663a362b480008659a65
😎 Deploy Preview	https://deploy-preview-2776--sublime-lsp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[predragnikolic]

Would be nice if docs were added for this feature. (does not have to be in this PR)