mattn / efm-langserver

General purpose Language Server
MIT License
1.32k stars 59 forks source link

There's no way to actually use the default value of `InitializationOptions` (inferred from the config file) since the zero value is false (that is, you must manually specify all the specs of the `InitializeOptions` in the lsp client) #250

Closed milanglacier closed 10 months ago

milanglacier commented 12 months ago

The zero value of a bool is false in go, which means the following lines:

if params.InitializationOptions != nil {
        hasCompletionCommand = params.InitializationOptions.Completion
        hasHoverCommand = params.InitializationOptions.Hover
        hasCodeActionCommand = params.InitializationOptions.CodeAction
        hasSymbolCommand = params.InitializationOptions.DocumentSymbol
        hasFormatCommand = params.InitializationOptions.DocumentFormatting
        hasRangeFormatCommand = params.InitializationOptions.RangeFormatting
    }

will always set the hasCompletionCommand, hasHoverCommand, etc. to false unless the user manually specify all the fields of InitializeOptions to true in the LSP client settings, which also means the code to infer the default value of those options will never be active since they will become false once params.InitializeOptions is read:

// These lines will never be effective
    for _, config := range h.configs {
        for _, v := range config {
            if v.CompletionCommand != "" {
                hasCompletionCommand = true
            }
            if v.HoverCommand != "" {
                hasHoverCommand = true
            }
            if v.SymbolCommand != "" {
                hasSymbolCommand = true
            }
            if v.FormatCommand != "" {
                hasFormatCommand = true
                if v.FormatCanRange {
                    hasRangeFormatCommand = true
                }
            }
        }
    }

This is a regression and also a breaking change since most user used v0.44. At that time the InitializeOptions was firstly read and then changed based on the config file. With the release of v0.46, the user must specify all the InitializeOptions in the LSP client settings.

llllvvuu commented 11 months ago

Sorry, it must have been my earlier PR somehow.

I'm not even sure how this was not broken before (maybe it was only accidentally working?), but anyways my new PR #256 improves the logic.