microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.19k stars 29.29k forks source link

false positive terminal launch failure notifications #160325

Open mfulton26 opened 2 years ago

mfulton26 commented 2 years ago

Type: Bug

using Ctrl+D to close a terminal where the last command exited with a non-zero exit code causes VSCode to notify the user of a terminal launch failure even though no failure occurred

e.g.

% echo hello | grep goodbye
% # press Ctrl+D

should I be closing terminal some other way? can VSCode detect the terminal being closed by the user and not report launch failures?

VS Code version: Code 1.71.0 (784b0177c56c607789f9638da7b6bf3230d47a8c, 2022-09-01T07:25:25.516Z) OS version: Darwin arm64 21.6.0 Modes: Sandboxed: No

System Info |Item|Value| |---|---| |CPUs|Apple M1 Pro (10 x 24)| |GPU Status|2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: disabled_off| |Load (avg)|2, 3, 3| |Memory (System)|16.00GB (0.07GB free)| |Process Argv|--crash-reporter-id 0132e674-27df-435e-9fa2-562b1adbdb94| |Screen Reader|no| |VM|0%|
Extensions (47) Extension|Author (truncated)|Version ---|---|--- cucumberautocomplete|ale|2.15.2 color-info|bie|0.7.0 comment-tagged-templates|bie|0.3.1 docs-view|bie|0.0.11 emojisense|bie|0.9.1 folder-source-actions|bie|0.2.1 gif-player|bie|0.0.2 github-markdown-preview|bie|0.3.0 jsdoc-markdown-highlighting|bie|0.0.1 markdown-checkbox|bie|0.3.2 markdown-emoji|bie|0.3.0 markdown-footnotes|bie|0.0.7 markdown-image-size|bie|0.0.4 markdown-mermaid|bie|1.15.2 markdown-preview-github-styles|bie|1.0.1 markdown-prism|bie|0.0.5 markdown-shiki|bie|0.1.1 markdown-yaml-preamble|bie|0.1.0 react-native-ios-pack|bie|0.0.3 speech|bie|0.1.0 scratchpads|bue|0.0.8 vscode-markdownlint|Dav|0.48.1 vscode-eslint|dba|2.2.6 vscode-deno|den|3.13.1 gitlens|eam|12.2.2 prettier-vscode|esb|9.8.0 vscode-pull-request-github|Git|0.50.0 vscode-graphql|Gra|0.7.4 vscode-graphql-syntax|Gra|1.0.4 rest-client|hum|0.25.1 swift|Kas|0.2.0 vscode-clang|mit|0.2.4 vscode-postgresql|ms-|0.3.0 live-server|ms-|0.4.0 test-adapter-converter|ms-|0.1.6 vsliveshare|ms-|1.0.5711 vsliveshare-audio|ms-|0.1.91 vsliveshare-pack|ms-|0.4.0 vscode-react-native|msj|1.9.2 vscode-xml|red|0.21.2022062916 vscode-yaml|red|1.10.1 stately-vscode|sta|1.9.3 code-spell-checker|str|2.7.2 es6-string-html|Tob|2.12.0 vscode-import-cost|wix|3.3.0 clang-format|xav|1.9.0 local-history|xyz|1.8.1
A/B Experiments ``` vsliv368cf:30146710 vsreu685:30147344 python383cf:30185419 vspor879:30202332 vspor708:30202333 vspor363:30204092 vslsvsres303:30308271 pythonvspyl392:30443607 vserr242cf:30382550 pythontb:30283811 vsjup518:30340749 pythonptprofiler:30281270 vshan820:30294714 vstes263:30335439 pythondataviewer:30285071 vscod805:30301674 binariesv615:30325510 bridge0708:30335490 bridge0723:30353136 cmake_vspar411:30557514 vsaa593cf:30376535 pythonvs932:30410667 cppdebug:30492333 vscaac:30438847 pylanb8912:30545647 vsclangdf:30486550 c4g48928:30535728 hb751961:30553087 dsvsc012cf:30540253 azure-dev_surveyone:30548225 2144e591:30553903 ```
meganrogge commented 2 years ago

when you send Ctrl D to the terminal, it causes the process to exit. The terminal receives the process exit event and can't tell the difference between what's coming from the user via the process and from the process directly.

Could you set up a keybinding to workbench.action.terminal.kill?

meganrogge commented 2 years ago

it is a little odd to me that we're using the last run command's exit code though 🤔

meganrogge commented 2 years ago

reproduces with and without shell integration enabled

Tyriar commented 2 years ago

On macOS this also happens if you run just exit in bash. The workaround here is to set "terminal.integrated.showExitAlert": false

mfulton26 commented 2 years ago

using "terminal.integrated.showExitAlert": false does work but it also means that I won't get alerted for actual terminal launch failures, plus it means every user who wants to use Ctrl+D without these notifications has to define this themselves

from what I understand, the integrated terminal already intercepts other key strokes; would it be feasible to intercept Ctrl+D so that VSCode still sends the signal but it knows to not notify about any terminal launch failures?

meganrogge commented 2 years ago

As I explained above, that would likely involve monitoring / scanning for input as we don't currently have a way to detect this case. Since there are viable workarounds, we don't want to complicate things

Tyriar commented 1 year ago

When shell integration is enabled we could actually improve upon this now by handling this specific case explicitly: on non-zero process exit, if mac, if the last command was exit, ignore it.

AlexanderOtavka commented 3 months ago

using "terminal.integrated.showExitAlert": false does work but it also means that I won't get alerted for actual terminal launch failures, plus it means every user who wants to use Ctrl+D without these notifications has to define this themselves

from what I understand, the integrated terminal already intercepts other key strokes; would it be feasible to intercept Ctrl+D so that VSCode still sends the signal but it knows to not notify about any terminal launch failures?

@mfulton26 There is actually a special exception for launch failures in the code. The pop-up will still be displayed if the terminal fails on launch, even if "terminal.integrated.showExitAlert" is set to false.

But you make a good point about each user needing to edit the setting. I think the setting is a bit difficult to find, and it would be easier if there was an option directly on the pop-up to say "Don't show again (except launch failures)".

mfulton26 commented 3 months ago

@AlexanderOtavka that would certainly be an improvement. I was not aware of the exception in the code, and it isn't documented from what I can tell. I think it would be good to mention this exception in the docs for "terminal.integrated.showExitAlert" (or maybe it is already there and I'm overlooking it).

I don't see any reason for VS Code to alert me when I use Ctrl+D though. I think automatically detecting that the terminal was closed via Ctrl+D and not considering such as a launch failure would be ideal (no initial pop-up to start ignoring, etc.).