config: Remove all code related to "static setting" (#307)

aviatesk · web-flow · commit f0260a56af20 · 2025-11-20T04:56:06.000+09:00
Our configuration system has had two concepts: "dynamic setting" and
"static setting". However, in practice, static settings were never
actually used.

Static settings require server restart, but such configurations should
actually be specified as `init_params.initializationOptions` and should
be handled in the initialization lifecycle.

For these reasons, the "static setting" concept in `ConfigManager` is
unnecessary, so this commit removes all related code.
diff --git a/src/config.jl b/src/config.jl
@@ -180,23 +180,11 @@ so `config` is guaranteed to not be `nothing`.
 """
 Base.@constprop :aggressive function get_config(manager::ConfigManager, key_path::Symbol...)
     data = load(manager)
-    config = if is_static_setting(key_path...)
-        getobjpath(data.__filled_static_settings__, key_path...)
-    else
-        getobjpath(data.__filled_settings__, key_path...)
-    end
+    config = getobjpath(data.__filled_settings__, key_path...)
     @assert !isnothing(config) "Invalid default configuration values"
     return config
 end
 
-function fix_static_settings!(manager::ConfigManager)
-    store!(manager) do old_data::ConfigManagerData
-        new_static = get_settings(old_data)
-        new_data = ConfigManagerData(old_data; static_settings=new_static)
-        return new_data, new_static
-    end
-end
-
 struct ConfigChange
     path::String
     old_val
@@ -206,19 +194,14 @@ end
 
 mutable struct ConfigChangeTracker
     changed_settings::Vector{ConfigChange}
-    changed_static_settings::Vector{ConfigChange}
     diagnostic_setting_changed::Bool
 end
-ConfigChangeTracker() = ConfigChangeTracker(ConfigChange[], ConfigChange[], false)
+ConfigChangeTracker() = ConfigChangeTracker(ConfigChange[], false)
 
 function (tracker::ConfigChangeTracker)(old_val, new_val, path::Tuple{Vararg{Symbol}})
     if old_val !== new_val
         path_str = join(path, ".")
-        if is_static_setting(path...)
-            push!(tracker.changed_static_settings, ConfigChange(path_str, old_val, new_val))
-        else
-            push!(tracker.changed_settings, ConfigChange(path_str, old_val, new_val))
-        end
+        push!(tracker.changed_settings, ConfigChange(path_str, old_val, new_val))
         if !isempty(path) && first(path) === :diagnostic
             tracker.diagnostic_setting_changed = true
         end
@@ -235,34 +218,12 @@ function changed_settings_message(changed_settings::Vector{ConfigChange})
     return "Changes applied: $body"
 end
 
-function changed_static_settings_message(changed_settings::Vector{ConfigChange})
-    body = map(changed_settings) do config_change
-        old_repr = repr(config_change.old_val)
-        new_repr = repr(config_change.new_val)
-        "`$(config_change.path)` (`$old_repr` => `$new_repr`)"
-    end |> (x -> join(x, ", "))
-    return "Static settings affected (requires restart to apply): $body"
-end
-
 function notify_config_changes(
         server::Server,
         tracker::ConfigChangeTracker,
         source::AbstractString
     )
-    if !isempty(tracker.changed_static_settings) && !isempty(tracker.changed_settings)
-        show_warning_message(server, """
-            Configuration changed.
-            Source: $source
-            $(changed_settings_message(tracker.changed_settings))
-            $(changed_static_settings_message(tracker.changed_static_settings))
-            """)
-    elseif !isempty(tracker.changed_static_settings)
-        show_warning_message(server, """
-            Configuration changed.
-            Source: $source
-            $(changed_static_settings_message(tracker.changed_static_settings))
-            """)
-    elseif !isempty(tracker.changed_settings)
+    if !isempty(tracker.changed_settings)
         show_info_message(server, """
             Configuration changed.
             Source: $source
diff --git a/src/initialize.jl b/src/initialize.jl
@@ -1,3 +1,8 @@
+# NOTE: Static server settings that require a server restart to take effect should be
+# accessed during server initialization via `state.init_params.initializationOptions`.
+# These settings differ from dynamic configuration options managed by `ConfigManager`
+# that can be changed at throughout server lifecycle.
+
 """
 Receives `msg::InitializeRequest` and sets up the `server.state` based on `msg.params`.
 As a response to this `msg`, it returns an `InitializeResponse` and performs registration of
@@ -268,9 +273,7 @@ function handle_InitializedNotification(server::Server)
         end
     end
     # - Load LSP configuration
-    #   In a case when client doesn't support the pull model configuration,
-    #   use `init_params.initializationOptions` as the fallback
-    load_lsp_config!(server, state.init_params.initializationOptions, "[LSP] initialize"; on_init=true)
+    load_lsp_config!(server, nothing, "[LSP] initialize"; on_init=true)
 
     registrations = Registration[]
 
diff --git a/src/types.jl b/src/types.jl
@@ -310,8 +310,6 @@ Note that `TypeX` should not be defined to include the possibility of being `not
 In such cases, a further inner configuration level should be used.
 
 Then, overload the following methods properly:
-- `is_static_setting(::Type{NewConfig}, field::Symbol) -> Bool`
-  Returns whether a setting requires server restart.
 - `default_config(::Type{NewConfig}) -> NewConfig`
   Returns the default configuration values.
 
@@ -325,15 +323,11 @@ _unwrap_maybe(::Type{T}) where {T} = T
 @option struct FullAnalysisConfig <: ConfigSection
     debounce::Maybe{Float64}
 end
-
-is_static_setting(::Type{FullAnalysisConfig}, ::Symbol) = false
 default_config(::Type{FullAnalysisConfig}) = FullAnalysisConfig(1.0)
 
 @option struct TestRunnerConfig <: ConfigSection
     executable::Maybe{String}
 end
-
-is_static_setting(::Type{TestRunnerConfig}, ::Symbol) = false
 default_config(::Type{TestRunnerConfig}) = TestRunnerConfig(@static Sys.iswindows() ? "testrunner.bat" : "testrunner")
 
 @option "custom" struct CustomFormatterConfig
@@ -342,9 +336,6 @@ default_config(::Type{TestRunnerConfig}) = TestRunnerConfig(@static Sys.iswindow
 end
 
 const FormatterConfig = Union{String,CustomFormatterConfig}
-
-is_static_setting(::Type{FormatterConfig}) = false
-is_static_setting(::Type{FormatterConfig}, ::Symbol) = false
 default_config(::Type{FormatterConfig}) = "Runic"
 
 function default_executable(formatter::String)
@@ -407,39 +398,20 @@ Base.convert(::Type{DiagnosticPattern}, x::AbstractDict{String}) =
     enabled::Maybe{Bool}
     patterns::Maybe{Vector{DiagnosticPattern}}
 end
-
-is_static_setting(::Type{DiagnosticConfig}, ::Symbol) = false
 default_config(::Type{DiagnosticConfig}) = DiagnosticConfig(true, DiagnosticPattern[])
 
-# configuration item for test purpose
-@option struct InternalConfig <: ConfigSection
-    static_setting::Maybe{Int}
-    dynamic_setting::Maybe{Int}
-end
-
-is_static_setting(::Type{InternalConfig}, field::Symbol) = field == :static_setting
-default_config(::Type{InternalConfig}) = InternalConfig(0, 0)
-
 @option struct JETLSConfig <: ConfigSection
     diagnostic::Maybe{DiagnosticConfig}
     full_analysis::Maybe{FullAnalysisConfig}
     testrunner::Maybe{TestRunnerConfig}
     formatter::Maybe{FormatterConfig}
-    internal::Maybe{InternalConfig}
 end
 
-is_static_setting(path::Symbol...) =
-    is_static_setting(JETLSConfig, path...)
-is_static_setting(::Type{<:ConfigSection}) = false
-is_static_setting(::Type{T}, head::Symbol, rest::Symbol...) where {T<:ConfigSection} =
-    is_static_setting(_unwrap_maybe(fieldtype(T, head)), rest...)
-
 const DEFAULT_CONFIG = JETLSConfig(
     diagnostic = default_config(DiagnosticConfig),
     full_analysis = default_config(FullAnalysisConfig),
     testrunner = default_config(TestRunnerConfig),
     formatter = default_config(FormatterConfig),
-    internal = default_config(InternalConfig)
 )
 
 function get_default_config(path::Symbol...)
@@ -451,15 +423,12 @@ end
 const EMPTY_CONFIG = JETLSConfig()
 
 struct ConfigManagerData
-    static_settings::JETLSConfig
     file_config::JETLSConfig
     lsp_config::JETLSConfig
     file_config_path::Union{Nothing,String}
     __settings__::JETLSConfig
-    __filled_static_settings__::JETLSConfig
     __filled_settings__::JETLSConfig
     function ConfigManagerData(
-            static_settings::JETLSConfig,
             file_config::JETLSConfig,
             lsp_config::JETLSConfig,
             file_config_path::Union{Nothing,String}
@@ -476,23 +445,21 @@ struct ConfigManagerData
         __settings__ = merge_setting(__settings__, lsp_config)
         __settings__ = merge_setting(__settings__, file_config)
         # Create setting structs without `nothing` values for use by `get_config`
-        __filled_static_settings__ = merge_setting(DEFAULT_CONFIG, static_settings)
         __filled_settings__ = merge_setting(DEFAULT_CONFIG, __settings__)
-        return new(static_settings, file_config, lsp_config, file_config_path,
-            __settings__, __filled_static_settings__, __filled_settings__)
+        return new(file_config, lsp_config, file_config_path,
+            __settings__, __filled_settings__)
     end
 end
 
-ConfigManagerData() = ConfigManagerData(DEFAULT_CONFIG, EMPTY_CONFIG, EMPTY_CONFIG, nothing)
+ConfigManagerData() = ConfigManagerData(EMPTY_CONFIG, EMPTY_CONFIG, nothing)
 
 function ConfigManagerData(
         data::ConfigManagerData;
-        static_settings::JETLSConfig = data.static_settings,
         file_config::JETLSConfig = data.file_config,
         lsp_config::JETLSConfig = data.lsp_config,
         file_config_path::Union{Nothing,String} = data.file_config_path
     )
-    return ConfigManagerData(static_settings, file_config, lsp_config, file_config_path)
+    return ConfigManagerData(file_config, lsp_config, file_config_path)
 end
 
 get_settings(data::ConfigManagerData) = data.__settings__
diff --git a/src/workspace-configuration.jl b/src/workspace-configuration.jl
@@ -13,7 +13,6 @@ function (handler::LoadLSPConfigHandler)(server::Server, @nospecialize(config_va
     end
     if handler.on_init
         # Don't notify even if values different from defaults are loaded initially
-        fix_static_settings!(server.state.config_manager)
     else
         notify_config_changes(handler.server, tracker, handler.source)
         if tracker.diagnostic_setting_changed
@@ -105,7 +104,6 @@ function load_lsp_config!(
         store_lsp_config!(tracker, server, settings, source)
         if on_init
             # Don't notify even if values different from defaults are loaded initially
-            fix_static_settings!(server.state.config_manager)
         else
             notify_config_changes(server, tracker, source)
             if tracker.diagnostic_setting_changed
diff --git a/test/test_config.jl b/test/test_config.jl
@@ -12,35 +12,24 @@ using JETLS
         @test_throws FieldError JETLS.get_default_config(:full_analysis, :nonexistent)
     end
 
-    @testset "`is_static_setting`" begin
-        @test !JETLS.is_static_setting(:internal, :dynamic_setting)
-        @test JETLS.is_static_setting(:internal, :static_setting)
-        @test !JETLS.is_static_setting(:testrunner, :executable)
-    end
-
     @testset "`merge_setting`" begin
         base_config = JETLS.JETLSConfig(;
             full_analysis=JETLS.FullAnalysisConfig(1.0),
             testrunner=JETLS.TestRunnerConfig("base_runner"),
-            internal=JETLS.InternalConfig(10, 20)
         )
         overlay_config = JETLS.JETLSConfig(;
             full_analysis=JETLS.FullAnalysisConfig(2.0),
-            internal=JETLS.InternalConfig(30, nothing)
         )
         merged = JETLS.merge_setting(base_config, overlay_config)
 
         @test JETLS.getobjpath(merged, :full_analysis, :debounce) == 2.0
         @test JETLS.getobjpath(merged, :testrunner, :executable) == "base_runner"
-        @test JETLS.getobjpath(merged, :internal, :static_setting) == 30
-        @test JETLS.getobjpath(merged, :internal, :dynamic_setting) == 20
     end
 
     @testset "`on_difference`" begin
         let config1 = JETLS.JETLSConfig(;
                 full_analysis=JETLS.FullAnalysisConfig(1.0),
                 testrunner=JETLS.TestRunnerConfig("runner1"),
-                internal=JETLS.InternalConfig(1, 1)
             )
             config2 = JETLS.JETLSConfig(;
                 full_analysis=JETLS.FullAnalysisConfig(2.0),
@@ -54,8 +43,6 @@ using JETLS
             @test Set(paths_called) == Set([
                 (:full_analysis, :debounce),
                 (:testrunner, :executable),
-                (:internal, :static_setting),
-                (:internal, :dynamic_setting) # even though new_val is nothing, track the path
             ])
         end
     end
@@ -131,23 +118,21 @@ end
     test_config = JETLS.JETLSConfig(;
         full_analysis=JETLS.FullAnalysisConfig(2.0),
         testrunner=JETLS.TestRunnerConfig("test_runner"),
-        internal=JETLS.InternalConfig(5, nothing)
     )
 
     store_file_config!(manager, "/foo/bar/.JETLSConfig.toml", test_config)
-    JETLS.fix_static_settings!(manager)
 
     @test JETLS.get_config(manager, :full_analysis, :debounce) === 2.0
     @test JETLS.get_config(manager, :testrunner, :executable) === "test_runner"
     @test_throws FieldError JETLS.get_config(manager, :nonexistent)
 
     # Type stability check (N.B: Nothing is not allowed)
     @test Base.infer_return_type((typeof(manager),)) do manager
-           JETLS.get_config(manager, :internal, :dynamic_setting)
-    end == Int
+           JETLS.get_config(manager, :full_analysis, :debounce)
+    end == Float64
     @test Base.infer_return_type((typeof(manager),)) do manager
-           JETLS.get_config(manager, :internal, :static_setting)
-    end == Int
+           JETLS.get_config(manager, :formatter)
+    end == JETLS.FormatterConfig
 
     # Test priority: file config has higher priority than LSP config
     lsp_config = JETLS.JETLSConfig(;
@@ -161,49 +146,13 @@ end
 
     # Test updating config
     store_lsp_config!(manager, JETLS.EMPTY_CONFIG)
-    changed_static_keys = Set{String}()
     updated_config = JETLS.JETLSConfig(;
         full_analysis=JETLS.FullAnalysisConfig(3.0),
         testrunner=JETLS.TestRunnerConfig("new_runner"),
-        internal=JETLS.InternalConfig(10, nothing)
     )
-    let data = JETLS.load(manager)
-        current_config = data.file_config
-        JETLS.on_difference(current_config, updated_config) do _, new_val, path
-            if JETLS.is_static_setting(JETLS.JETLSConfig, path...)
-                push!(changed_static_keys, join(path, "."))
-            end
-            return new_val
-        end
-    end
     store_file_config!(manager, "/foo/bar/.JETLSConfig.toml", updated_config)
-
-    # `on_static_setting` should be called for static keys
-    @test changed_static_keys == Set(["internal.static_setting"])
-    # non static keys should be changed dynamically
-    @test JETLS.get_config(manager, :testrunner, :executable) == "new_runner"
     @test JETLS.get_config(manager, :full_analysis, :debounce) == 3.0
-    # static keys should NOT change (they stay at the fixed values)
-    @test JETLS.get_config(manager, :internal, :static_setting) == 5
-end
-
-@testset "`fix_static_settings!`" begin
-    manager = JETLS.ConfigManager(JETLS.ConfigManagerData())
-    file_config = JETLS.JETLSConfig(;
-        internal=JETLS.InternalConfig(2, nothing)
-    )
-    lsp_config = JETLS.JETLSConfig(;
-        internal=JETLS.InternalConfig(999, nothing),
-        testrunner=JETLS.TestRunnerConfig("custom")
-    )
-
-    store_file_config!(manager, "/path/.JETLSConfig.toml", file_config)
-    store_lsp_config!(manager, lsp_config)
-    JETLS.fix_static_settings!(manager)
-
-    # File config (higher priority) should win for the static keys
-    data = JETLS.load(manager)
-    @test JETLS.getobjpath(data.static_settings, :internal, :static_setting) == 2
+    @test JETLS.get_config(manager, :testrunner, :executable) == "new_runner"
 end
 
 @testset "LSP configuration priority and merging" begin
@@ -229,14 +178,11 @@ end
 
 @testset "LSP configuration merging without file config" begin
     manager = JETLS.ConfigManager(JETLS.ConfigManagerData())
-
     lsp_config = JETLS.JETLSConfig(;
         full_analysis=JETLS.FullAnalysisConfig(3.0),
         testrunner=JETLS.TestRunnerConfig("lsp_runner")
     )
-
     store_lsp_config!(manager, lsp_config)
-
     @test JETLS.get_config(manager, :testrunner, :executable) == "lsp_runner"
     @test JETLS.get_config(manager, :full_analysis, :debounce) == 3.0
 end
diff --git a/test/test_diagnostics.jl b/test/test_diagnostics.jl
@@ -215,7 +215,7 @@ end
 
 function make_test_manager(config_dict::Dict{String,Any})
     lsp_config = JETLS.Configurations.from_dict(JETLS.JETLSConfig, config_dict)
-    data = JETLS.ConfigManagerData(JETLS.DEFAULT_CONFIG, JETLS.EMPTY_CONFIG, lsp_config, nothing)
+    data = JETLS.ConfigManagerData(JETLS.EMPTY_CONFIG, lsp_config, nothing)
     return JETLS.ConfigManager(data)
 end
 
diff --git a/test/test_did-change-watched-files.jl b/test/test_did-change-watched-files.jl
