Skip to content

Commit a535688

Browse files
authored
fix(codewhisperer): impove 'hover' override robustness for inline suggestions (#2902)
## Problem CodeWhisperer won't restore user settings if they reload/exit at certain times ## Solution Use globalState + restore the setting on extension activation
1 parent eb63856 commit a535688

File tree

3 files changed

+32
-4
lines changed

3 files changed

+32
-4
lines changed

src/codewhisperer/activation.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ import {
5757
const performance = globalThis.performance ?? require('perf_hooks').performance
5858

5959
export async function activate(context: ExtContext): Promise<void> {
60+
// No need to await. This can be removed once the 'hover.enabled' hack is no longer needed.
61+
HoverConfigUtil.instance.restoreHoverConfig().catch(err => {
62+
getLogger().warn('codewhisperer: failed to restore "editor.hover.enabled" setting: %O', err)
63+
})
64+
6065
const codewhispererSettings = CodeWhispererSettings.instance
6166
if (!codewhispererSettings.isEnabled()) {
6267
return

src/codewhisperer/util/hoverConfigUtil.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,17 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6+
import globals from '../../shared/extensionGlobals'
67
import { Settings } from '../../shared/settings'
78

89
export class HoverConfigUtil extends Settings.define('editor', { 'hover.enabled': Boolean }) {
910
// disable hover popup when inline suggestion is active
1011
// this is because native popup Next/Previous button have bug that removes active inline suggestion
1112
// this class can be removed once inlineCompletionAdditions API is available
12-
private userHoverEnabled?: boolean
13+
14+
public constructor(private readonly globalState = globals.context.globalState, settings = Settings.instance) {
15+
super(settings)
16+
}
1317

1418
static #instance: HoverConfigUtil
1519

@@ -19,14 +23,15 @@ export class HoverConfigUtil extends Settings.define('editor', { 'hover.enabled'
1923

2024
async overwriteHoverConfig() {
2125
if (this.get('hover.enabled', false)) {
26+
await this.globalState.update('settings.editor.hover.enabled', true)
2227
await this.update('hover.enabled', false)
23-
this.userHoverEnabled = true
2428
}
2529
}
2630

2731
async restoreHoverConfig() {
28-
if (this.userHoverEnabled) {
29-
await this.update('hover.enabled', this.userHoverEnabled)
32+
if (this.globalState.get('settings.editor.hover.enabled')) {
33+
await this.update('hover.enabled', true)
34+
await this.globalState.update('settings.editor.hover.enabled', undefined)
3035
}
3136
}
3237
}

src/test/codewhisperer/util/hoverConfigUtil.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import * as assert from 'assert'
77
import { HoverConfigUtil } from '../../../codewhisperer/util/hoverConfigUtil'
8+
import { FakeMemento } from '../../fakeExtensionContext'
89

910
describe('HoverConfigUtil', function () {
1011
describe('overwriteHoverConfig', async function () {
@@ -15,6 +16,7 @@ describe('HoverConfigUtil', function () {
1516
const actual = hoverConfigUtil.get('hover.enabled', false)
1617
assert.strictEqual(actual, false)
1718
})
19+
1820
it('Should not set hover enabled to false if it is currently false', async function () {
1921
const hoverConfigUtil = new HoverConfigUtil()
2022
await hoverConfigUtil.update('hover.enabled', false)
@@ -33,6 +35,7 @@ describe('HoverConfigUtil', function () {
3335
const actual = hoverConfigUtil.get('hover.enabled', false)
3436
assert.strictEqual(actual, true)
3537
})
38+
3639
it('Should not restore hover config it was not previously overwritten', async function () {
3740
const hoverConfigUtil = new HoverConfigUtil()
3841
await hoverConfigUtil.update('hover.enabled', false)
@@ -41,4 +44,19 @@ describe('HoverConfigUtil', function () {
4144
assert.strictEqual(actual, false)
4245
})
4346
})
47+
48+
it('stores the previous setting value in the provided memento', async function () {
49+
const memento = new FakeMemento()
50+
51+
const util1 = new HoverConfigUtil(memento)
52+
await util1.update('hover.enabled', true)
53+
await util1.overwriteHoverConfig()
54+
55+
const util2 = new HoverConfigUtil(memento)
56+
await util2.update('hover.enabled', false)
57+
await util2.restoreHoverConfig()
58+
59+
const actual = util2.get('hover.enabled', false)
60+
assert.strictEqual(actual, true)
61+
})
4462
})

0 commit comments

Comments
 (0)