fix: respect refactoring_auto_save in more cases

[rchl]

code_actions.py

• the logic should be unchanged here. I just had to extract the "kind includes other kind" logic out of this code and place it somewhere where Session could access it.

protocol.py

• added kind_includes_other_kind function

sessions.py

• Fixed a bug where a refactoring code action would not always trigger save with refactoring_auto_save enabled. It only worked if the code action had refactoring kind exactly and not if it had more specific one like refactoring.move. This is fixed by the new kind_includes_other_kind function.
• Allow passing is_refactoring to Session.execute_command to allow for all edits triggered within this request to be treated as refactoring and trigger the refactoring_auto_save logic. (see fix(deps): update dependency typescript-language-server to v5 LSP-typescript#278 to see the code that uses that)

[jwortmann]

This description is somewhat misleading. I would say "refactor.rewrite" is a subset of "refactor" (because "refactor.rewrite" is more specific). So it's the other way round. It's a bit confusing because the "bigger" kind string is a substring of the more specific kind string.

[rchl]

I struggle with describing this properly. Hard to make it clear without also describing the whole code action matching logic and how code actions are structured...

[rchl]

Actually the naming for this function might be wrong/confusing.

In JS the language-server libraries have similar logic and they have contains function which actually does the opposite of this function.

    /**
     * Checks if `other` is a sub-kind of this `CodeActionKind`.
     *
     * The kind `"refactor.extract"` for example contains `"refactor.extract"` and ``"refactor.extract.function"`,
     * but not `"unicorn.refactor.extract"`, or `"refactor.extractAll"` or `refactor`.
     *
     * @param other Kind to check.
     */
    public contains(other: CodeActionKind): boolean {
        return this.equals(other) || this.value === '' || other.value.startsWith(this.value + CodeActionKind.sep);
    }


CodeActionKind.QuickFix.contains('quickfix.foo') === true

[rchl]

Changed the code to match those libs.

[jwortmann]

I think the "includes" was from the previous function which had an array of kinds for the input.

Maybe rename like this

def is_subkind(base_kind: str, kind: str) -> bool:
    """
    Return `True` if `kind` is a subkind of `base_kind`.
    For example "a.b" is a subkind of "a" and of "a.b".
    """

[rchl]

Are you happy with matching the Microsoft's libraries?

I find the short is_subkind not ideal for two reasons:

• it doesn't make it clear that it's related to code action kinds unless you see the parameters
• "subkind" is a bit confusing wording in case of kinds. One would think that "a" is a subkind of "a.b" (in the sense that "a.b" includes "a" when thinking about it as a string) but it's the opposite.

[jwortmann]

Yeah it's okay. But could you move this function out of protocol.py? In my opinion ideally this file should only contain the auto-generated types from the specs (I know there is some other stuff too, but let's not keep it growing if possible). Maybe put it in the "utils" file views.py or in sessions.py.

[rchl]

utils is exactly what I was planning to do first but was a bit afraid of creating a file for any random stuff. Alternatively was also thinking about creating core/code_actions but then there would be two code_actions.

Maybe I'm overthinking it.

[jwortmann]

We already have the utils file, it is just named views.py. So let's just use that for now.

Perhaps some time in the future we can do a bigger refactoring of the files structure. But first we need a proper API for plugins (what currently is LSP/plugin/__init__.py, but then including all of the functionality that can be used by plugins - and then fix all the imports in plugins that currently import some random things from various files directly). And then I would also move (only) the auto-generated types from LSP/plugin/core/protocol.py to LSP/protocol/__init__.py and add a line from ...protocol import * into LSP/plugin/core/protocol.py for backward compatibility. So that we have a clean didvision between "official" LSP protocol types and the custom classes and types defined by this package. Then there would only be two API "endpoints" for imports in helper plugins: e.g. from LSP.protocol import TextDocumentPositionParams for the LSP types and from LSP.plugin import Request for all other things.