Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dim terminal when app terminates #17780

Open
elad-eyal-ns opened this issue Aug 22, 2024 · 6 comments
Open

Dim terminal when app terminates #17780

elad-eyal-ns opened this issue Aug 22, 2024 · 6 comments
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Milestone

Comments

@elad-eyal-ns
Copy link

I have a profile which is used for SSH. When the connection is lost, I get a message saying the process has terminated and I am to press ENTER to restart. It's not always easy to spot this message, since it can appear at the top or middle of the screen (wherever the cursor happened to be). I propose that at this time (optionally) dim the entire window. This way it will be easier to notice the app termination and hit ENTER before continuing.

@elad-eyal-ns elad-eyal-ns added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Aug 22, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Aug 22, 2024
@carlos-zamora carlos-zamora added this to the Backlog milestone Sep 4, 2024
@carlos-zamora carlos-zamora added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 4, 2024
@zadjii-msft zadjii-msft modified the milestones: Backlog, Icebox ❄ Sep 4, 2024
@DHowett
Copy link
Member

DHowett commented Sep 4, 2024

Thanks for the request! For what it's worth... we have another visual indicator that the connection has been broken:

Image

We're not likely to prioritize the "appearance matrix" work required to support the combinations of {terminated/running, admin/non-admin, focused/unfocused} required to make this possible, so we're committing it to the distant distant backlog for now.

@zadjii-msft
Copy link
Member

and my stupid idea from triage:

  • connection dead? ---> deadConnectionAppearance
  • alive?
    • unfocused? ---> unfocusedAppearance
    • focused?
      • admin? ---> adminAppearance
      • standard ---> defaultAppearance

@elad-eyal
Copy link

Thanks for the request! For what it's worth... we have another visual indicator that the connection has been broken:

Image

what is "another visual indicator" ? the two lines of text? when using ssh and tmux they tend to appear anywhere on screen, hiding between whatever text happens to be on screen. it is not as clear as in the demo image you placed.

@DHowett
Copy link
Member

DHowett commented Sep 5, 2024

Good point.

It’s the (x) which appears to the left of the title. It’s visible in the tab.

@hi2u
Copy link

hi2u commented Feb 6, 2025

I posted a similar issue #18489 a few days ago, didn't find this one here at the time.

@carlos-zamora's reply in #18489:

A consistent problem we've been running into is that we need a way to resolve combined states (i.e. admin session that is unfocused).

@DHowett's reply here:

We're not likely to prioritize the "appearance matrix" work required to support the combinations

Yeah fair enough, that could be over-complicating it.

For each of these "binary states" we're doubling the total number of combined states for the whole program. And I don't think it should be dictated how the prioritization works anyway. Especially when there's the possibilities of having 3+ of these competing binary states... 3 binary states is already 8 combined states seeing we're doubling for each one... and maybe more will come in the future.

People have their own priorities, and they might even have differing priorities amongst their own profiles.

Maybe the simplest + most flexible solution would just to be to use an array. i.e. rather than unfocusedAppearance etc being keys in the top-level profile object, they're just a type field in an array of objects, i.e.

OLD (what we have now)

{
    "name": "My Profile Name",
    "commandline": "the_command.exe",
    "guid": "{23516c80-6c27-41ec-9049-4a56b1511a5d}",
    "background": "#000000",
    "unfocusedAppearance": {
        "background": "#680909",
    }
},

NEW

{
    "name": "My Profile Name",
    "commandline": "the_command.exe",
    "guid": "{23516c80-6c27-41ec-9049-4a56b1511a5d}",
    "appearances": [
        {
            "type": "defaultAppearance",
            "background": "#000000",
        },
        {
            "type": "unfocusedAppearance",
            "background": "#680909",
        },
        {
            "type": "deadProcessAppearance",
            "background": "#838996",
        },
    ]
},

All of the currently applicable "binary states" in the appearances array are merged, and any "competing" overlapping settings such as background will "win" based on either being first or last matching state in the array (pretty similar to how firewall rules get applied).

"Last match wins" probably makes more sense to me seeing it's merging settings rather than doing a "hard stop" that only applies one of the states that ignores the rest that also apply. And you'll usually have "default" at the top, and you get more "niche" as you read downward. Pretty easy to read, and probably simpler to code too (JS example below).

This would give users full flexibility for their own priorities, including when they differ for different profiles. And is pretty future-proof if more of these "binary states" are added later.

Whether or not defaultAppearance is included in the array, or just remains top-level settings is a separate decision I guess. Not really needed for this request in general, but could be considered later on if you wanted to clean up all the properties in the top-level.

Shouldn't be too hard to code hopefully either? It's just looping through an array and overwriting applicable values in the final merged appearance object. I know Terminal is written in C++, but just an an example of how it would be done pretty easily in JS with Array.reduce()....

@zadjii-msft
Copy link
Member

Moving a comment out of the triage thread

Image

appearances with when field. the user solves the combinatorial matrix. also actions with a when field like VS Code. some of our whens are provided by default. one of the whens could be connectionTerminated ... and

  • appearance + connectionTerminated = change color when process dies
  • action + connectionTerminated = move ^D and Enter actions out of ConptyConnection

some keywords to find this thread faster next time: conditional appearance combinatorial appearances matrix focused admin unfocused dead connection theme light dark

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

6 participants