Support resetting settings to the initial state

Author: eliericha

doc/settings.md

@@ -2,11 +2,11 @@
 
 ## Configuration Sources
 
-The ALS loads configuration settings from different sources in the following order:
+The ALS loads configuration settings from different sources. All configuration sources are loaded in the following order:
 
-1. The file `$XDG_CONFIG_HOME/als/config.json`, if it exists.
+1. A global user configuration file `$XDG_CONFIG_HOME/als/config.json`, if it exists.
 
-1. The file `.als.json` in the directory where ALS is spawned, if it exists.
+1. A workspace-specific `.als.json` file in the directory where ALS is spawned, if it exists.
 
    This is the prefered location to store project-specific settings that are tracked in version control and shared among developers.
 
@@ -16,7 +16,11 @@ The ALS loads configuration settings from different sources in the following ord
 
 1. In `workspace/didChangeConfiguration` LSP notifications, if specified.
 
-If given, the configuration file must be a JSON file with the following structure:
+Each configuration source can contain a partial list of settings. Thus each
+configuration source can override individual settings while preserving
+previously loaded settings.
+
+Configuration files must be JSON files matching [this JSON schema](integration/vscode/ada/schemas/als-settings-schema.json). Roughly the structure looks like this:
 
 ```json
 {
@@ -61,6 +65,12 @@ Similarly, settings passed in `workspace/didChangeConfiguration` notifications s
 }
 ```
 
+## Base Configuration
+
+The *base configuration* is the one that ALS reaches after loading configuration files (i.e. global user configuration, workspace-specific `.als.json` file, and `--config` command line argument).
+
+After that the ALS may receive configuration changes through the `initialize` request, or the `workspace/didChangeConfiguration` notification. In those messages, if settings have the value `null`, ALS reverts their value to the base configuration. This allows clients to temporarily override settings, and revert them back in the same session.
+
 ## Visual Studio Code
 
 In the context of Visual Studio Code, configuration settings can be set in the

integration/vscode/ada/test/dot-als-json/dot_als_json.test.ts

@@ -1,14 +1,17 @@
 import assert from 'assert';
-import { adaExtState } from '../../src/extension';
-import { activate } from '../utils';
 import path from 'path';
 import { ConfigurationTarget, workspace } from 'vscode';
+import { adaExtState } from '../../src/extension';
+import { activate } from '../utils';
 
 suite('dot-als-json', function () {
     this.beforeAll(async function () {
         await activate();
     });
 
+    const alsDotJsonValue = 'als-dot-json-value';
+    const settingsDotJsonValue = 'settings-json-value';
+
     /**
      * The goal of this testsuite is to verify the interaction between settings
      * coming from .als.json and .vscode/settings.json.
@@ -35,24 +38,22 @@ suite('dot-als-json', function () {
          * Initially the scenario variable is set in .als.json but not in settings.json
          */
         const objDir = path.basename(await adaExtState.getObjectDir());
-        const expected = 'als-dot-json-value';
+        const expected = alsDotJsonValue;
         assert.equal(objDir, expected);
     });
 
     test('case-2', async function () {
         const initial = workspace.getConfiguration('ada').get('scenarioVariables');
         try {
-            /**
-             * First we change the VS Code settings to override the scenario variable.
-             */
-            const sentinel = 'settings-json-value';
-            await workspace.getConfiguration('ada').update('scenarioVariables', { Var: sentinel });
+            await workspace
+                .getConfiguration('ada')
+                .update('scenarioVariables', { Var: settingsDotJsonValue });
 
             /**
              * Check that the change overwrote the .als.json value
              */
             const objDir = path.basename(await adaExtState.getObjectDir());
-            assert.equal(objDir, sentinel);
+            assert.equal(objDir, settingsDotJsonValue);
         } finally {
             /**
              * Restore the initial value of the setting.
@@ -70,33 +71,26 @@ suite('dot-als-json', function () {
     test('undo-override', async function () {
         const initial = workspace.getConfiguration('ada').get('scenarioVariables');
         try {
-            /**
-             * First we change the VS Code settings to override the scenario variable.
-             */
-            const sentinel = 'settings-json-value';
-            await workspace.getConfiguration('ada').update('scenarioVariables', { Var: sentinel });
+            await workspace
+                .getConfiguration('ada')
+                .update('scenarioVariables', { Var: settingsDotJsonValue });
 
             /**
              * Check that the change overwrote the .als.json value
              */
             let objDir = path.basename(await adaExtState.getObjectDir());
-            assert.equal(objDir, sentinel);
+            assert.equal(objDir, settingsDotJsonValue);
 
             /**
              * Now let's undo the override.
              */
             await workspace.getConfiguration('ada').update('scenarioVariables', undefined);
 
             /**
-             * Ideally we would want the ALS to switch back to the setting from
-             * .als.json but this is not the current behavior.
-             *
-             * Currently the didChangeConfiguration notification contains \{
-             * "scenarioVariables": null \} which makes ALS ignore the provided
-             * value and retain the last stored value.
+             * The value should go back to the .als.json value
              */
             objDir = path.basename(await adaExtState.getObjectDir());
-            assert.equal(objDir, sentinel);
+            assert.equal(objDir, alsDotJsonValue);
         } finally {
             /**
              * Restore the initial value of the setting.

source/ada/lsp-ada_did_change_configurations.adb

@@ -15,9 +15,11 @@
 -- of the license.                                                          --
 ------------------------------------------------------------------------------
 
+with GPR2.Context;
 with LSP.Ada_Configurations;
 with LSP.Client_Message_Receivers;
 with LSP.Server_Notifications.DidChangeConfiguration;
+with VSS.Strings;
 
 package body LSP.Ada_Did_Change_Configurations is
 
@@ -75,24 +77,61 @@ package body LSP.Ada_Did_Change_Configurations is
       Message : LSP.Server_Messages.Server_Message_Access)
       return LSP.Server_Jobs.Server_Job_Access
    is
+      use type VSS.Strings.Virtual_String;
+      use type GPR2.Context.Object;
+
       Value : LSP.Server_Notifications.DidChangeConfiguration.Notification
         renames LSP.Server_Notifications.DidChangeConfiguration.Notification
           (Message.all);
 
       Result : constant Apply_Config_Job_Access :=
         new Apply_Config_Job (Self'Unchecked_Access);
 
-      Reload : Boolean renames Result.Reload;
-   begin
-      Result.Configuration :=
+      Base_Config : constant LSP.Ada_Configurations.Configuration :=
+        LSP.Ada_Configurations.Configuration
+          (Self.Context.Get_Base_Configuration.all);
+
+      Current_Config : constant LSP.Ada_Configurations.Configuration :=
         LSP.Ada_Configurations.Configuration
           (Self.Context.Get_Configuration.all);
 
-      Result.Configuration.Read_JSON (Value.Params.settings, Reload);
+      New_Config : LSP.Ada_Configurations.Configuration
+        renames Result.Configuration;
+      Reload : Boolean renames Result.Reload;
+   begin
+
+      --  The configuration loading logic offers a Reload output boolean that
+      --  indicates if values were changed that require reloading the project.
+      --  However this doesn't work when a 'null' value is received. 'null'
+      --  values are ignored by the configuration parser and thus don't signal
+      --  the Reload flag.
+      --
+      --  To address that we have to compare the relevant configuration
+      --  settings after they are parsed and cannot rely on the configuration
+      --  parsing logic.
+
+      --  Start with the base configuration.
+      New_Config := Base_Config;
+
+      --  Read the new received configuration. 'null' values will be ignored
+      --  and thus keep the value from the base config.
+      New_Config.Read_JSON (Value.Params.settings, Reload);
 
       --  Always reload project if Project_Tree isn't ready
       Reload := Reload or not Self.Context.Project_Tree_Is_Defined;
 
+      --  Compare with the current configuration to see if a reload is
+      --  necessary
+      Reload :=
+        Reload
+        or else New_Config.Relocate_Build_Tree
+                /= Current_Config.Relocate_Build_Tree
+        or else New_Config.Relocate_Root /= Current_Config.Relocate_Root
+        or else New_Config.Project_File /= Current_Config.Project_File
+        or else New_Config.Context /= Current_Config.Context
+        or else New_Config.Charset /= Current_Config.Charset
+        or else New_Config.Follow_Symlinks /= Current_Config.Follow_Symlinks;
+
       if Reload then
          --  Stop indexing by changing project stamp
          Self.Context.Increment_Project_Timestamp;

source/ada/lsp-ada_handlers.adb

@@ -434,6 +434,11 @@ package body LSP.Ada_Handlers is
          Self.Client.Set_Root_If_Empty (
             To_Virtual_String (ALS_Workspace_Config_File.Get_Parent));
       end if;
+
+      --  Save the initial configuration so that we can restore individual
+      --  settings back to the initial state when 'onDidChangeConfiguration'
+      --  provides null values.
+      Self.Base_Configuration := Self.Configuration;
    end Load_Config_Files;
 
    -------------------

source/ada/lsp-ada_handlers.ads

@@ -177,7 +177,17 @@ private
      and LSP.Ada_Job_Contexts.Ada_Job_Context
    with record
       Client : aliased LSP.Ada_Client_Capabilities.Client_Capability;
+
+      Base_Configuration : aliased LSP.Ada_Configurations.Configuration;
+      --  This is the initial configuration loaded at process startup from
+      --  configuration files (global, local .als.json, CLI --config). Later
+      --  on the configuration may be overwritten by the 'initialize' request
+      --  or 'onDidChangeConfiguration' notifications so saving this initial
+      --  configuration allows to revert settings back when a null value is
+      --  received in an `onDidChangeConfiguration` notification.
+
       Configuration : aliased LSP.Ada_Configurations.Configuration;
+      --  The current configuration in use.
 
       Contexts : LSP.Ada_Context_Sets.Context_Set;
       --  There is one context in this list per loaded project.
@@ -424,6 +434,10 @@ private
      access constant LSP.Ada_Client_Capabilities.Client_Capability'Class
        is (Self.Client'Unchecked_Access);
 
+   overriding function Get_Base_Configuration (Self : Message_Handler)
+     return access constant LSP.Ada_Configurations.Configuration'Class
+       is (Self.Base_Configuration'Unchecked_Access);
+
    overriding function Get_Configuration (Self : Message_Handler)
      return access constant LSP.Ada_Configurations.Configuration'Class is
        (Self.Configuration'Unchecked_Access);

source/ada/lsp-ada_job_contexts.ads

@@ -53,9 +53,16 @@ package LSP.Ada_Job_Contexts is
      access constant LSP.Ada_Client_Capabilities.Client_Capability'Class
        is abstract;
 
+   function Get_Base_Configuration (Self : Ada_Job_Context)
+     return access constant LSP.Ada_Configurations.Configuration'Class
+       is abstract;
+   --  The base configuration loaded at process startup from configuration
+   --  files.
+
    function Get_Configuration (Self : Ada_Job_Context)
      return access constant LSP.Ada_Configurations.Configuration'Class
        is abstract;
+   --  The current applicable configuration.
 
    procedure Set_Configuration
       (Self  : in out Ada_Job_Context;