mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-20 18:14:29 -07:00
feat(cli): replace loading phrases boolean with enum setting (#19347)
This commit is contained in:
@@ -2032,6 +2032,85 @@ describe('Settings Loading and Merging', () => {
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
// Check that enableLoadingPhrases: false was further migrated to loadingPhrases: 'off'
|
||||
expect(setValueSpy).toHaveBeenCalledWith(
|
||||
SettingScope.User,
|
||||
'ui',
|
||||
expect.objectContaining({
|
||||
loadingPhrases: 'off',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should migrate enableLoadingPhrases: false to loadingPhrases: off', () => {
|
||||
const userSettingsContent = {
|
||||
ui: {
|
||||
accessibility: {
|
||||
enableLoadingPhrases: false,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const loadedSettings = createMockSettings(userSettingsContent);
|
||||
const setValueSpy = vi.spyOn(loadedSettings, 'setValue');
|
||||
|
||||
migrateDeprecatedSettings(loadedSettings);
|
||||
|
||||
expect(setValueSpy).toHaveBeenCalledWith(
|
||||
SettingScope.User,
|
||||
'ui',
|
||||
expect.objectContaining({
|
||||
loadingPhrases: 'off',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should not migrate enableLoadingPhrases: true to loadingPhrases', () => {
|
||||
const userSettingsContent = {
|
||||
ui: {
|
||||
accessibility: {
|
||||
enableLoadingPhrases: true,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const loadedSettings = createMockSettings(userSettingsContent);
|
||||
const setValueSpy = vi.spyOn(loadedSettings, 'setValue');
|
||||
|
||||
migrateDeprecatedSettings(loadedSettings);
|
||||
|
||||
// Should not set loadingPhrases when enableLoadingPhrases is true
|
||||
const uiCalls = setValueSpy.mock.calls.filter((call) => call[1] === 'ui');
|
||||
for (const call of uiCalls) {
|
||||
const uiValue = call[2] as Record<string, unknown>;
|
||||
expect(uiValue).not.toHaveProperty('loadingPhrases');
|
||||
}
|
||||
});
|
||||
|
||||
it('should not overwrite existing loadingPhrases during migration', () => {
|
||||
const userSettingsContent = {
|
||||
ui: {
|
||||
loadingPhrases: 'witty',
|
||||
accessibility: {
|
||||
enableLoadingPhrases: false,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const loadedSettings = createMockSettings(userSettingsContent);
|
||||
const setValueSpy = vi.spyOn(loadedSettings, 'setValue');
|
||||
|
||||
migrateDeprecatedSettings(loadedSettings);
|
||||
|
||||
// Should not overwrite existing loadingPhrases
|
||||
const uiCalls = setValueSpy.mock.calls.filter((call) => call[1] === 'ui');
|
||||
for (const call of uiCalls) {
|
||||
const uiValue = call[2] as Record<string, unknown>;
|
||||
if (uiValue['loadingPhrases'] !== undefined) {
|
||||
expect(uiValue['loadingPhrases']).toBe('witty');
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('should prioritize new settings over deprecated ones and respect removeDeprecated flag', () => {
|
||||
|
||||
@@ -165,7 +165,10 @@ export interface SummarizeToolOutputSettings {
|
||||
tokenBudget?: number;
|
||||
}
|
||||
|
||||
export type LoadingPhrasesMode = 'tips' | 'witty' | 'all' | 'off';
|
||||
|
||||
export interface AccessibilitySettings {
|
||||
/** @deprecated Use ui.loadingPhrases instead. */
|
||||
enableLoadingPhrases?: boolean;
|
||||
screenReader?: boolean;
|
||||
}
|
||||
@@ -928,6 +931,22 @@ export function migrateDeprecatedSettings(
|
||||
anyModified = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Migrate enableLoadingPhrases: false → loadingPhrases: 'off'
|
||||
const enableLP = newAccessibility['enableLoadingPhrases'];
|
||||
if (
|
||||
typeof enableLP === 'boolean' &&
|
||||
newUi['loadingPhrases'] === undefined
|
||||
) {
|
||||
if (!enableLP) {
|
||||
newUi['loadingPhrases'] = 'off';
|
||||
loadedSettings.setValue(scope, 'ui', newUi);
|
||||
if (!settingsFile.readOnly) {
|
||||
anyModified = true;
|
||||
}
|
||||
}
|
||||
foundDeprecated.push('ui.accessibility.enableLoadingPhrases');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -83,6 +83,19 @@ describe('SettingsSchema', () => {
|
||||
).toBe('boolean');
|
||||
});
|
||||
|
||||
it('should have loadingPhrases enum property', () => {
|
||||
const definition = getSettingsSchema().ui?.properties?.loadingPhrases;
|
||||
expect(definition).toBeDefined();
|
||||
expect(definition?.type).toBe('enum');
|
||||
expect(definition?.default).toBe('tips');
|
||||
expect(definition?.options?.map((o) => o.value)).toEqual([
|
||||
'tips',
|
||||
'witty',
|
||||
'all',
|
||||
'off',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should have checkpointing nested properties', () => {
|
||||
expect(
|
||||
getSettingsSchema().general?.properties?.checkpointing.properties
|
||||
|
||||
@@ -672,6 +672,22 @@ const SETTINGS_SCHEMA = {
|
||||
description: 'Show the spinner during operations.',
|
||||
showInDialog: true,
|
||||
},
|
||||
loadingPhrases: {
|
||||
type: 'enum',
|
||||
label: 'Loading Phrases',
|
||||
category: 'UI',
|
||||
requiresRestart: false,
|
||||
default: 'tips',
|
||||
description:
|
||||
'What to show while the model is working: tips, witty comments, both, or nothing.',
|
||||
showInDialog: true,
|
||||
options: [
|
||||
{ value: 'tips', label: 'Tips' },
|
||||
{ value: 'witty', label: 'Witty' },
|
||||
{ value: 'all', label: 'All' },
|
||||
{ value: 'off', label: 'Off' },
|
||||
],
|
||||
},
|
||||
customWittyPhrases: {
|
||||
type: 'array',
|
||||
label: 'Custom Witty Phrases',
|
||||
@@ -700,8 +716,9 @@ const SETTINGS_SCHEMA = {
|
||||
category: 'UI',
|
||||
requiresRestart: true,
|
||||
default: true,
|
||||
description: 'Enable loading phrases during operations.',
|
||||
showInDialog: true,
|
||||
description:
|
||||
'@deprecated Use ui.loadingPhrases instead. Enable loading phrases during operations.',
|
||||
showInDialog: false,
|
||||
},
|
||||
screenReader: {
|
||||
type: 'boolean',
|
||||
|
||||
@@ -1596,6 +1596,8 @@ Logging in with Google... Restarting Gemini CLI to continue.
|
||||
streamingState,
|
||||
shouldShowFocusHint,
|
||||
retryStatus,
|
||||
loadingPhrasesMode: settings.merged.ui.loadingPhrases,
|
||||
customWittyPhrases: settings.merged.ui.customWittyPhrases,
|
||||
});
|
||||
|
||||
const handleGlobalKeypress = useCallback(
|
||||
|
||||
@@ -391,16 +391,16 @@ describe('Composer', () => {
|
||||
expect(output).not.toContain('ShortcutsHint');
|
||||
});
|
||||
|
||||
it('renders LoadingIndicator without thought when accessibility disables loading phrases', async () => {
|
||||
it('renders LoadingIndicator without thought when loadingPhrases is off', async () => {
|
||||
const uiState = createMockUIState({
|
||||
streamingState: StreamingState.Responding,
|
||||
thought: { subject: 'Hidden', description: 'Should not show' },
|
||||
});
|
||||
const config = createMockConfig({
|
||||
getAccessibility: vi.fn(() => ({ enableLoadingPhrases: false })),
|
||||
const settings = createMockSettings({
|
||||
merged: { ui: { loadingPhrases: 'off' } },
|
||||
});
|
||||
|
||||
const { lastFrame } = await renderComposer(uiState, undefined, config);
|
||||
const { lastFrame } = await renderComposer(uiState, settings);
|
||||
|
||||
const output = lastFrame();
|
||||
expect(output).toContain('LoadingIndicator');
|
||||
|
||||
@@ -211,12 +211,12 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => {
|
||||
thought={
|
||||
uiState.streamingState ===
|
||||
StreamingState.WaitingForConfirmation ||
|
||||
config.getAccessibility()?.enableLoadingPhrases === false
|
||||
settings.merged.ui.loadingPhrases === 'off'
|
||||
? undefined
|
||||
: uiState.thought
|
||||
}
|
||||
currentLoadingPhrase={
|
||||
config.getAccessibility()?.enableLoadingPhrases === false
|
||||
settings.merged.ui.loadingPhrases === 'off'
|
||||
? undefined
|
||||
: uiState.currentLoadingPhrase
|
||||
}
|
||||
@@ -255,12 +255,12 @@ export const Composer = ({ isFocused = true }: { isFocused?: boolean }) => {
|
||||
thought={
|
||||
uiState.streamingState ===
|
||||
StreamingState.WaitingForConfirmation ||
|
||||
config.getAccessibility()?.enableLoadingPhrases === false
|
||||
settings.merged.ui.loadingPhrases === 'off'
|
||||
? undefined
|
||||
: uiState.thought
|
||||
}
|
||||
currentLoadingPhrase={
|
||||
config.getAccessibility()?.enableLoadingPhrases === false
|
||||
settings.merged.ui.loadingPhrases === 'off'
|
||||
? undefined
|
||||
: uiState.currentLoadingPhrase
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ export const INFORMATIVE_TIPS = [
|
||||
'Show memory usage for performance monitoring (/settings)…',
|
||||
'Show line numbers in the chat for easier reference (/settings)…',
|
||||
'Show citations to see where the model gets information (/settings)…',
|
||||
'Disable loading phrases for a quieter experience (/settings)…',
|
||||
'Customize loading phrases: tips, witty, all, or off (/settings)…',
|
||||
'Add custom witty phrases to the loading screen (settings.json)…',
|
||||
'Use alternate screen buffer to preserve shell history (/settings)…',
|
||||
'Choose a specific Gemini model for conversations (/settings)…',
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
import { WITTY_LOADING_PHRASES } from '../constants/wittyPhrases.js';
|
||||
import { INFORMATIVE_TIPS } from '../constants/tips.js';
|
||||
import type { RetryAttemptPayload } from '@google/gemini-cli-core';
|
||||
import type { LoadingPhrasesMode } from '../../config/settings.js';
|
||||
|
||||
describe('useLoadingIndicator', () => {
|
||||
beforeEach(() => {
|
||||
@@ -33,21 +34,25 @@ describe('useLoadingIndicator', () => {
|
||||
initialStreamingState: StreamingState,
|
||||
initialShouldShowFocusHint: boolean = false,
|
||||
initialRetryStatus: RetryAttemptPayload | null = null,
|
||||
loadingPhrasesMode: LoadingPhrasesMode = 'all',
|
||||
) => {
|
||||
let hookResult: ReturnType<typeof useLoadingIndicator>;
|
||||
function TestComponent({
|
||||
streamingState,
|
||||
shouldShowFocusHint,
|
||||
retryStatus,
|
||||
mode,
|
||||
}: {
|
||||
streamingState: StreamingState;
|
||||
shouldShowFocusHint?: boolean;
|
||||
retryStatus?: RetryAttemptPayload | null;
|
||||
mode?: LoadingPhrasesMode;
|
||||
}) {
|
||||
hookResult = useLoadingIndicator({
|
||||
streamingState,
|
||||
shouldShowFocusHint: !!shouldShowFocusHint,
|
||||
retryStatus: retryStatus || null,
|
||||
loadingPhrasesMode: mode,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
@@ -56,6 +61,7 @@ describe('useLoadingIndicator', () => {
|
||||
streamingState={initialStreamingState}
|
||||
shouldShowFocusHint={initialShouldShowFocusHint}
|
||||
retryStatus={initialRetryStatus}
|
||||
mode={loadingPhrasesMode}
|
||||
/>,
|
||||
);
|
||||
return {
|
||||
@@ -68,7 +74,8 @@ describe('useLoadingIndicator', () => {
|
||||
streamingState: StreamingState;
|
||||
shouldShowFocusHint?: boolean;
|
||||
retryStatus?: RetryAttemptPayload | null;
|
||||
}) => rerender(<TestComponent {...newProps} />),
|
||||
mode?: LoadingPhrasesMode;
|
||||
}) => rerender(<TestComponent mode={loadingPhrasesMode} {...newProps} />),
|
||||
};
|
||||
};
|
||||
|
||||
@@ -221,4 +228,15 @@ describe('useLoadingIndicator', () => {
|
||||
expect(result.current.currentLoadingPhrase).toContain('Trying to reach');
|
||||
expect(result.current.currentLoadingPhrase).toContain('Attempt 3/3');
|
||||
});
|
||||
|
||||
it('should show no phrases when loadingPhrasesMode is "off"', () => {
|
||||
const { result } = renderLoadingIndicatorHook(
|
||||
StreamingState.Responding,
|
||||
false,
|
||||
null,
|
||||
'off',
|
||||
);
|
||||
|
||||
expect(result.current.currentLoadingPhrase).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -12,11 +12,13 @@ import {
|
||||
getDisplayString,
|
||||
type RetryAttemptPayload,
|
||||
} from '@google/gemini-cli-core';
|
||||
import type { LoadingPhrasesMode } from '../../config/settings.js';
|
||||
|
||||
export interface UseLoadingIndicatorProps {
|
||||
streamingState: StreamingState;
|
||||
shouldShowFocusHint: boolean;
|
||||
retryStatus: RetryAttemptPayload | null;
|
||||
loadingPhrasesMode?: LoadingPhrasesMode;
|
||||
customWittyPhrases?: string[];
|
||||
}
|
||||
|
||||
@@ -24,6 +26,7 @@ export const useLoadingIndicator = ({
|
||||
streamingState,
|
||||
shouldShowFocusHint,
|
||||
retryStatus,
|
||||
loadingPhrasesMode,
|
||||
customWittyPhrases,
|
||||
}: UseLoadingIndicatorProps) => {
|
||||
const [timerResetKey, setTimerResetKey] = useState(0);
|
||||
@@ -37,6 +40,7 @@ export const useLoadingIndicator = ({
|
||||
isPhraseCyclingActive,
|
||||
isWaiting,
|
||||
shouldShowFocusHint,
|
||||
loadingPhrasesMode,
|
||||
customWittyPhrases,
|
||||
);
|
||||
|
||||
|
||||
@@ -14,23 +14,27 @@ import {
|
||||
} from './usePhraseCycler.js';
|
||||
import { INFORMATIVE_TIPS } from '../constants/tips.js';
|
||||
import { WITTY_LOADING_PHRASES } from '../constants/wittyPhrases.js';
|
||||
import type { LoadingPhrasesMode } from '../../config/settings.js';
|
||||
|
||||
// Test component to consume the hook
|
||||
const TestComponent = ({
|
||||
isActive,
|
||||
isWaiting,
|
||||
isInteractiveShellWaiting = false,
|
||||
loadingPhrasesMode = 'all',
|
||||
customPhrases,
|
||||
}: {
|
||||
isActive: boolean;
|
||||
isWaiting: boolean;
|
||||
isInteractiveShellWaiting?: boolean;
|
||||
loadingPhrasesMode?: LoadingPhrasesMode;
|
||||
customPhrases?: string[];
|
||||
}) => {
|
||||
const phrase = usePhraseCycler(
|
||||
isActive,
|
||||
isWaiting,
|
||||
isInteractiveShellWaiting,
|
||||
loadingPhrasesMode,
|
||||
customPhrases,
|
||||
);
|
||||
return <Text>{phrase}</Text>;
|
||||
@@ -289,6 +293,7 @@ describe('usePhraseCycler', () => {
|
||||
<TestComponent
|
||||
isActive={config.isActive}
|
||||
isWaiting={false}
|
||||
loadingPhrasesMode="witty"
|
||||
customPhrases={config.customPhrases}
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
import { useState, useEffect, useRef } from 'react';
|
||||
import { INFORMATIVE_TIPS } from '../constants/tips.js';
|
||||
import { WITTY_LOADING_PHRASES } from '../constants/wittyPhrases.js';
|
||||
import type { LoadingPhrasesMode } from '../../config/settings.js';
|
||||
|
||||
export const PHRASE_CHANGE_INTERVAL_MS = 15000;
|
||||
export const INTERACTIVE_SHELL_WAITING_PHRASE =
|
||||
@@ -17,23 +18,20 @@ export const INTERACTIVE_SHELL_WAITING_PHRASE =
|
||||
* @param isActive Whether the phrase cycling should be active.
|
||||
* @param isWaiting Whether to show a specific waiting phrase.
|
||||
* @param shouldShowFocusHint Whether to show the shell focus hint.
|
||||
* @param customPhrases Optional list of custom phrases to use.
|
||||
* @param loadingPhrasesMode Which phrases to show: tips, witty, all, or off.
|
||||
* @param customPhrases Optional list of custom phrases to use instead of built-in witty phrases.
|
||||
* @returns The current loading phrase.
|
||||
*/
|
||||
export const usePhraseCycler = (
|
||||
isActive: boolean,
|
||||
isWaiting: boolean,
|
||||
shouldShowFocusHint: boolean,
|
||||
loadingPhrasesMode: LoadingPhrasesMode = 'tips',
|
||||
customPhrases?: string[],
|
||||
) => {
|
||||
const loadingPhrases =
|
||||
customPhrases && customPhrases.length > 0
|
||||
? customPhrases
|
||||
: WITTY_LOADING_PHRASES;
|
||||
|
||||
const [currentLoadingPhrase, setCurrentLoadingPhrase] = useState<
|
||||
string | undefined
|
||||
>(isActive ? loadingPhrases[0] : undefined);
|
||||
>(undefined);
|
||||
|
||||
const phraseIntervalRef = useRef<NodeJS.Timeout | null>(null);
|
||||
const hasShownFirstRequestTipRef = useRef(false);
|
||||
@@ -55,30 +53,43 @@ export const usePhraseCycler = (
|
||||
return;
|
||||
}
|
||||
|
||||
if (!isActive) {
|
||||
if (!isActive || loadingPhrasesMode === 'off') {
|
||||
setCurrentLoadingPhrase(undefined);
|
||||
return;
|
||||
}
|
||||
|
||||
const wittyPhrases =
|
||||
customPhrases && customPhrases.length > 0
|
||||
? customPhrases
|
||||
: WITTY_LOADING_PHRASES;
|
||||
|
||||
const setRandomPhrase = () => {
|
||||
if (customPhrases && customPhrases.length > 0) {
|
||||
const randomIndex = Math.floor(Math.random() * customPhrases.length);
|
||||
setCurrentLoadingPhrase(customPhrases[randomIndex]);
|
||||
} else {
|
||||
let phraseList;
|
||||
// Show a tip on the first request after startup, then continue with 1/6 chance
|
||||
if (!hasShownFirstRequestTipRef.current) {
|
||||
// Show a tip during the first request
|
||||
let phraseList: readonly string[];
|
||||
|
||||
switch (loadingPhrasesMode) {
|
||||
case 'tips':
|
||||
phraseList = INFORMATIVE_TIPS;
|
||||
hasShownFirstRequestTipRef.current = true;
|
||||
} else {
|
||||
// Roughly 1 in 6 chance to show a tip after the first request
|
||||
const showTip = Math.random() < 1 / 6;
|
||||
phraseList = showTip ? INFORMATIVE_TIPS : WITTY_LOADING_PHRASES;
|
||||
}
|
||||
const randomIndex = Math.floor(Math.random() * phraseList.length);
|
||||
setCurrentLoadingPhrase(phraseList[randomIndex]);
|
||||
break;
|
||||
case 'witty':
|
||||
phraseList = wittyPhrases;
|
||||
break;
|
||||
case 'all':
|
||||
// Show a tip on the first request after startup, then continue with 1/6 chance
|
||||
if (!hasShownFirstRequestTipRef.current) {
|
||||
phraseList = INFORMATIVE_TIPS;
|
||||
hasShownFirstRequestTipRef.current = true;
|
||||
} else {
|
||||
const showTip = Math.random() < 1 / 6;
|
||||
phraseList = showTip ? INFORMATIVE_TIPS : wittyPhrases;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
phraseList = INFORMATIVE_TIPS;
|
||||
break;
|
||||
}
|
||||
|
||||
const randomIndex = Math.floor(Math.random() * phraseList.length);
|
||||
setCurrentLoadingPhrase(phraseList[randomIndex]);
|
||||
};
|
||||
|
||||
// Select an initial random phrase
|
||||
@@ -95,7 +106,13 @@ export const usePhraseCycler = (
|
||||
phraseIntervalRef.current = null;
|
||||
}
|
||||
};
|
||||
}, [isActive, isWaiting, shouldShowFocusHint, customPhrases, loadingPhrases]);
|
||||
}, [
|
||||
isActive,
|
||||
isWaiting,
|
||||
shouldShowFocusHint,
|
||||
loadingPhrasesMode,
|
||||
customPhrases,
|
||||
]);
|
||||
|
||||
return currentLoadingPhrase;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user