From 92bb4624c4ce191ed5dd60aaca4e2786039b3b2b Mon Sep 17 00:00:00 2001
From: Ali Al Jufairi <ali.redha.ali@gmail.com>
Date: Tue, 19 Aug 2025 11:28:45 +0900
Subject: feat(settings): enhance settings management with generic setter and
 display hel… (#6202)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Jacob Richman <jacob314@gmail.com>
---
 packages/cli/src/utils/settingsUtils.test.ts |  4 ++--
 packages/cli/src/utils/settingsUtils.ts      | 34 ++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 11 deletions(-)

(limited to 'packages/cli/src/utils')

diff --git a/packages/cli/src/utils/settingsUtils.test.ts b/packages/cli/src/utils/settingsUtils.test.ts
index 2aeb1da3..98a4588c 100644
--- a/packages/cli/src/utils/settingsUtils.test.ts
+++ b/packages/cli/src/utils/settingsUtils.test.ts
@@ -392,7 +392,7 @@ describe('SettingsUtils', () => {
           new Set(),
           updatedPendingSettings,
         );
-        expect(displayValue).toBe('true*'); // Should show true with * indicating change
+        expect(displayValue).toBe('true'); // Should show true (no * since value matches default)
 
         // Test that modified settings also show the * indicator
         const modifiedSettings = new Set([key]);
@@ -602,7 +602,7 @@ describe('SettingsUtils', () => {
           mergedSettings,
           modifiedSettings,
         );
-        expect(result).toBe('false'); // matches default, no *
+        expect(result).toBe('false*');
       });
 
       it('should show default value when setting is not in scope', () => {
diff --git a/packages/cli/src/utils/settingsUtils.ts b/packages/cli/src/utils/settingsUtils.ts
index f4363400..6b5a55c1 100644
--- a/packages/cli/src/utils/settingsUtils.ts
+++ b/packages/cli/src/utils/settingsUtils.ts
@@ -91,7 +91,10 @@ export function getRestartRequiredSettings(): string[] {
 /**
  * Recursively gets a value from a nested object using a key path array.
  */
-function getNestedValue(obj: Record<string, unknown>, path: string[]): unknown {
+export function getNestedValue(
+  obj: Record<string, unknown>,
+  path: string[],
+): unknown {
   const [first, ...rest] = path;
   if (!first || !(first in obj)) {
     return undefined;
@@ -332,6 +335,20 @@ export function setPendingSettingValue(
   return newSettings;
 }
 
+/**
+ * Generic setter: Set a setting value (boolean, number, string, etc.) in the pending settings
+ */
+export function setPendingSettingValueAny(
+  key: string,
+  value: unknown,
+  pendingSettings: Settings,
+): Settings {
+  const path = key.split('.');
+  const newSettings = structuredClone(pendingSettings);
+  setNestedValue(newSettings, path, value);
+  return newSettings;
+}
+
 /**
  * Check if any modified settings require a restart
  */
@@ -382,11 +399,9 @@ export function saveModifiedSettings(
       // We need to set the whole parent object.
       const [parentKey] = path;
       if (parentKey) {
-        // Ensure value is a boolean for setPendingSettingValue
-        const booleanValue = typeof value === 'boolean' ? value : false;
-        const newParentValue = setPendingSettingValue(
+        const newParentValue = setPendingSettingValueAny(
           settingKey,
-          booleanValue,
+          value,
           loadedSettings.forScope(scope).settings,
         )[parentKey as keyof Settings];
 
@@ -431,11 +446,12 @@ export function getDisplayValue(
   const isChangedFromDefault =
     typeof defaultValue === 'boolean' ? value !== defaultValue : value === true;
   const isInModifiedSettings = modifiedSettings.has(key);
-  const hasPendingChanges =
-    pendingSettings && settingExistsInScope(key, pendingSettings);
 
-  // Add * indicator when value differs from default, is in modified settings, or has pending changes
-  if (isChangedFromDefault || isInModifiedSettings || hasPendingChanges) {
+  // Mark as modified if setting exists in current scope OR is in modified settings
+  if (settingExistsInScope(key, settings) || isInModifiedSettings) {
+    return `${valueString}*`; // * indicates setting is set in current scope
+  }
+  if (isChangedFromDefault || isInModifiedSettings) {
     return `${valueString}*`; // * indicates changed from default value
   }
 
-- 
cgit v1.2.3