Terminal environment indicator is way too subtle, low contrast and overlaps text

[DanTup]

This indicator seems like a really important part of this new feature. If users don't notice it, it will seem like the feature doesn't work.

The icon is grey, really low contrast against the background, and text can run over/under it making it even harder to see:

Since this only appears when an extension has done something fairly important (like you just changed SDK and it updated the path), I think it should be a lot more visible than this.

[Tyriar]

It's subtle by design, the overlap is also intentional as mouse events pass through to the terminal. I was actually concerned it might be too obvious.

Since this only appears when an extension has done something fairly important

This isn't necessarily the case though, for example the js-debug extension setting up node debugging or git auth, both of which the user may want to use, maybe not. In general I don't want the user to be interrupted unless I'm sure something is critical. Also this will appear for every new workspace.

cc @misolori

[DanTup]

This isn't necessarily the case though, for example the js-debug extension setting up node debugging or git auth, both of which the user may want to use, maybe not. In general I don't want the user to be interrupted unless I'm sure something is critical

But if the user doesn't notice the icon and just tries to do this anyway, it's just going to seem like it doesn't work?

I've had bugs raised that when the SDK is changed on the status bar, the terminal doesn't update. User expect this to "just work". If I release support for it, I'm certain that close to 0% of users will notice that icon or realise they need to click on it for it to work (whilst it would be documented and included in release notes, I've come to learn that nobody reads them and they just expect software to do the obvious thing).

Also this will appear for every new workspace.

Then I also think there should be some ability to remember the users choice/apply to all. If I choose to trust an extension to manipulate my path, I probably shouldn't have to keep clicking a barely-noticable icon on each project load.

I hoped this feature would bring convenience and avoid confusion with mismatched SDK versions, but as it is right now, I wouldn't ship it. I think it would cause more confusion.

I understand there may be implications to allow silent manipulating the users path, but we're running extensions that already have pretty much full access to the host machine (at least the one running the extension host). We can probably read the existing PATH and write our own binaries into folders that are in it if we wanted to hijack commands. It feels like the experience is significantly compromised for not much gain in terms of security.

[DanTup]

Alternatively - would it be a reasonable feature request to let us hijack specific commands from the terminal? Eg. I could register dart and flutter and the first time a user types them into the terminal, it prompts that an extension would like to handle those commands (and we can just manipulate the path of the executable and give it back to VS Code/terminal to execute)? Doing that subsequently without a prompt (and for any workspaces) seems less risky as the user has basically said the extension can handle these commands without prompting.

[Tyriar]

Then I also think there should be some ability to remember the users choice/apply to all. If I choose to trust an extension to manipulate my path, I probably shouldn't have to keep clicking a barely-noticable icon on each project load.

It's not a matter of security, it's a matter of the extension host not being up for the extension to give valid paths for the workspace, and whether to use it work this workspace at all. We cannot block the terminal launch on the extension host as it would then often take 5+ seconds to open.

Alternatively - would it be a reasonable feature request to let us hijack specific commands from the terminal?

That's completely out of our hands, the only way to do that is to change PATH and/or create a dart/flutter in a higher priority bin directory.

[Tyriar]

After we get terminal tabs, the design will probably change for the warning to be inline in the tab, though there is still the issue of the tab containing multiple terminals and potentially only some of them being stale.

How about a workaround to make the warning a solid foreground color instead? It's not as much of an issue now that the default for terminal.integrated.environmentChangesIndicator is warnonly.

[DanTup]

It's not a matter of security, it's a matter of the extension host not being up for the extension to give valid paths for the workspace, and whether to use it work this workspace at all. We cannot block the terminal launch on the extension host as it would then often take 5+ seconds to open.

I thought that's why the value was cached? Maybe I misunderstood the comment above - if the user accepts, is the value applied automatically without the icon next time?

Alternatively - would it be a reasonable feature request to let us hijack specific commands from the terminal?

That's completely out of our hands, the only way to do that is to change PATH and/or create a dart/flutter in a higher priority bin directory.

Sorry, I mean by VS Code handling it in an API. Eg. we request permissions from the user to handle "dart" and "flutter" terminal commands, and when the user types something starting with those in the terminal, they get passed to an extension API to manipulate before being executed.

Using PATH would also be my preference, but the subtle icon doesn't seem like it'll work very well - users are very unlikely to notice it.

How about a workaround to make the warning a solid foreground color instead?

Anything to make it more obvious would be a benefit IMO. Honestly, I'd paint a big strip across the top of the terminal with obvious text ("An extension would like to modify the terminal environment") rather than a mystery icon the user has to hover over. And if the terminal hasn't been interacted with at load time, can't it just be recycled automatically to avoid the prompt?

[Tyriar]

Maybe I misunderstood the comment above - if the user accepts, is the value applied automatically without the icon next time?

It's cached by workspace as a lot of this usage would be workspace specific.

Sorry, I mean by VS Code handling it in an API. Eg. we request permissions from the user to handle "dart" and "flutter" terminal commands, and when the user types something starting with those in the terminal, they get passed to an extension API to manipulate before being executed.

Interesting idea, but this what I mean by it's out of our hands is something like this is unexpected from the user and hacky imo.

And if the terminal hasn't been interacted with at load time

This is a idea that I liked, I don't think there's an issue tracking it yet though.

[DanTup]

It's cached by workspace as a lot of this usage would be workspace specific.

Maybe it'd be useful to be able to set at both levels? For example if at startup, the Dart extension finds a user-setting that has an SDK path, it'd be reasonable to set that globally. If it's found in a workspace-specific setting, it could be set just for the workspace.

That way, loading a new project could use the global cache and avoid the user having to click buttons to fix the terminal for projects using their "default" SDK.

Interesting idea, but this what I mean by it's out of our hands is something like this is unexpected from the user and hacky imo.

It doesn't feel hacky to me. Based on reports I've had, I think users expect extensions to work better with the terminal than they do. For example people sometimes run flutter run in the built-in terminal and then are surprised that they can't do Hot Reload (because the extension has no connection to the process they started). I thought I'd seen an open issue about doing something in this area, but I can't find it now.

This is a idea that I liked, I don't think there's an issue tracking it yet though.

I've opened one to track it then :-) #100193

[Tyriar]

It doesn't feel hacky to me. Based on reports I've had, I think users expect extensions to work better with the terminal than they do. For example people sometimes run flutter run in the built-in terminal and then are surprised that they can't do Hot Reload (because the extension has no connection to the process they started). I thought I'd seen an open issue about doing something in this area, but I can't find it now.

Well if prepending to PATH works then you can do this as an extension anyway, it's would essentially just be a convenience method on top of this API.