[target] Don't set $FUCHSIA_NODENAME for terminals This causes confusion and leads to a poor DX when switching devices frequently (users have to know to pay attention to the warning icon and click "relaunch terminal window"). In addition to this, the IDE terminal window gets out of sync with non-IDE terminal windows. Upon initial implementation of this feature this was an intentional design choice, but now it is causing user complaint. This allows a more harmonious use-case of being able to not set default targets while only one device is connected. Delete $FUCHSIA_NODENAME from the extension's terminal environment variable contributions upon startup to purge values set by the previous version of the extension. Bug: 409568762 Change-Id: Ic7eaba7a606171ca0e5d12285c0b1be714f42400 Reviewed-on: https://fuchsia-review.googlesource.com/c/vscode-plugins/+/1247967 Reviewed-by: Amy Hu <amyhu@google.com> Reviewed-by: Clayton Wilkinson <wilkinsonclay@google.com> Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/extension.ts b/src/extension.ts index 961b3a5..cfab028 100644 --- a/src/extension.ts +++ b/src/extension.ts
@@ -42,16 +42,8 @@ */ fx: Fx; - constructor(ctx: vscode.ExtensionContext) { - // Initialize ffx with the default target as `$FUCHSIA_NODENAME` - // corresponding to the default device from the last VSCode session. - // - // This allows the extension be in sync with terminal sessions and avoid - // race conditions if both are activated upon initialization when VSCode is - // opened. - const previousSessionDefaultTarget = ctx?.environmentVariableCollection.get('FUCHSIA_NODENAME')?.value; - - this.toolFinder = new ToolFinder(previousSessionDefaultTarget); + constructor() { + this.toolFinder = new ToolFinder(); this.ffx = this.toolFinder.ffx; this.fx = this.toolFinder.fx; } @@ -62,6 +54,12 @@ * (see package.json for activation points) */ export async function activate(ctx: vscode.ExtensionContext) { + // Clean up old $FUCHSIA_NODENAME terminal environment variable contributions. + // Do this early since this extension's `activate()` call can race against + // terminal spawns on IDE startup. + // See https://fxbug.dev/409568762 for more information. + ctx.environmentVariableCollection.delete('FUCHSIA_NODENAME'); + // Initialize the logging first so it can be used by all other code. const log = vscode.window.createOutputChannel('Fuchsia Extension', { log: true }); logger.initLogger(log); @@ -76,7 +74,7 @@ logger.error('Unable to set up analytics, boldly continuing', undefined, err); }; - let setup = new Setup(ctx); + const setup = new Setup(); // TODO(fxbug.dev/98651): we're currently doing this every program // start -- we should find a better activation point @@ -102,8 +100,6 @@ setUpFuchsiaTaskProvider(ctx); setUpAnalyticsEvents(ctx); - - await setUpDefaultTargetTerminalInteraction(ctx, setup); } /** @@ -239,35 +235,3 @@ // ...and the status bar item new TargetStatusBarItem(ctx.subscriptions, setup.toolFinder); } - -/** - * initializes the ffx IDE default target and allows the values to be propagated - * as ffx default targets to IDE terminal instances. - */ -async function setUpDefaultTargetTerminalInteraction(ctx: vscode.ExtensionContext, setup: Setup) { - try { - // Ffx commands might take a bit longer during initialization. - await setup.ffx.refreshTargets(5000); - } catch (e) { - logger.error('Unable to refresh devices', 'setUpDefaultTargetTerminalInteraction', e); - } - - let lastDevice: string | undefined; - const propagateFuchsiaNodename = (device: FuchsiaDevice | null) => { - if (lastDevice === device?.nodeName) { - return; - } - - lastDevice = device?.nodeName; - if (lastDevice) { - ctx.environmentVariableCollection.replace('FUCHSIA_NODENAME', lastDevice, { - applyAtProcessCreation: true, - applyAtShellIntegration: true, - }); - } else { - ctx.environmentVariableCollection.delete('FUCHSIA_NODENAME'); - } - }; - propagateFuchsiaNodename(setup.ffx.targetDevice); - setup.ffx.onSetTarget(propagateFuchsiaNodename); -}
diff --git a/src/ffx.ts b/src/ffx.ts index c07eb00..9333ee7 100644 --- a/src/ffx.ts +++ b/src/ffx.ts
@@ -87,17 +87,12 @@ constructor( cwd: string | undefined, ffxPath?: string, - defaultTarget?: string, ) { this.spawnOptions = cwd ? { cwd: cwd } : {}; this.pathInternal = ffxPath; this.ffxPathChangedEvent = new vscode.EventEmitter<FfxEventType>(); this.onDidChangeConfiguration = this.ffxPathChangedEvent.event; this.onSetTarget = this.targetChangedEvent.event; - - if (defaultTarget) { - this.defaultTarget = FuchsiaDevice.fromNodename(defaultTarget); - } } /**
diff --git a/src/tool_finder.ts b/src/tool_finder.ts index b429b43..34710f8 100644 --- a/src/tool_finder.ts +++ b/src/tool_finder.ts
@@ -29,10 +29,10 @@ */ public readonly onDidUpdateFfx = this.onDidUpdateFfxEmitter.event; - constructor(ffxDefaultTarget?: string) { + constructor() { const cwd = vscode.workspace.workspaceFolders?.map(folder => folder.uri.path)[0]; - this.ffxInternal = new Ffx(cwd, undefined, ffxDefaultTarget); + this.ffxInternal = new Ffx(cwd, undefined); this.fxInternal = new Fx(cwd, this.ffxInternal); this.updateFfxPath(true).catch((err) => logger.error('unable to configure initial ffx location', undefined, err)); vscode.workspace.onDidChangeConfiguration(event => {