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

Automatically populate icons for the associated profiles #705

Closed
Tracked by #13392
JBanks opened this issue May 11, 2019 · 9 comments · Fixed by #15843
Closed
Tracked by #13392

Automatically populate icons for the associated profiles #705

JBanks opened this issue May 11, 2019 · 9 comments · Fixed by #15843
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Work-Item It's being tracked by an actual work item internally. (to be removed soon)

Comments

@JBanks
Copy link
Contributor

JBanks commented May 11, 2019

  • Your Windows build number:
    Microsoft Windows [Version 10.0.18362.86]

  • What you're doing and what's happening:
    Opening the terminal for the first time does not automatically select icons for the profiles

  • What's wrong / what should be happening instead:
    Icons should be automatically placed in the profile for use in the flyout, tab list, and jump list

@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Work-Item It's being tracked by an actual work item internally. (to be removed soon) labels May 13, 2019
@zadjii-msft zadjii-msft added this to the Windows Terminal v1.0 milestone May 13, 2019
@zadjii-msft
Copy link
Member

This is something that I've wanted to do, but just haven't got the chance to yet.

This was previously MSFT: 21097504
Here are some notes I had before:

https://stackoverflow.com/a/1520223
https://docs.microsoft.com/en-us/windows/desktop/api/shellapi/nf-shellapi-shgetfileinfoa
https://docs.microsoft.com/en-us/windows/desktop/api/shellapi/ns-shellapi-_shfileinfoa

We can get the HICON from an exe. Theoretically, there should be a way to turn that icon into a bitmap, and a way to load that bitmap as an image that UWP XAML can use.

That Icon could be used in the new tab button's flyout.

this WON'T work for WSL distros - they're Reparse points, so their exe's don't have ico's associated with them, the app does.

@Biswa96
Copy link

Biswa96 commented May 13, 2019

so their exe's don't have ico's associated with them, the app does

Did you check the EXE files in WSL Appx packages?

@zadjii-msft
Copy link
Member

@Biswa96 It's possible that'll work. You'd probably have to navigate through the reparse point to find the real exe when you encounter one. It's certainly possible to do, though I don't know how to.

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 17, 2019
@miniksa miniksa added Product-Terminal The new Windows Terminal. Area-User Interface Issues pertaining to the user interface of the Console or Terminal and removed Mass-Chaos labels May 17, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 17, 2019
@zadjii-msft
Copy link
Member

This is highly related to #1504

@zadjii-msft
Copy link
Member

You know, in retrospect, automatically guessing the icon from the commandline is probably a bad idea. Something like pwsh -nologo and now we've got to try and find the actual executable in the commandline - not something I want us to be in the business of.

#1504 is the better solution here - allowing the user to manually declare that they'd like to use an exe for the icon. It's less "magic", but it's more robust. So we'll just focus on that aspect of this request for now.

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Nov 11, 2021
@DHowett
Copy link
Member

DHowett commented Nov 11, 2021

So, I'll provide the counteropinion on this one. I think it's the right thing to do.

Because we can be the default terminal on Windows, we will want to derive icons from any LNK files¹ and EXE files that were used to spawn the console. Building on that: we may want to port any information we can from those files to create ephemeral profiles -- color schemes, cursor styles, etc. -- so that users who switch won't be quite so jarred that we've forgotten all their settings.

¹which allow you to specify a custom icon, and a bunch of internal developers do just that; I have to assume it's common practice outside Microsoft as well 😄

@DHowett DHowett reopened this Nov 11, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 11, 2021
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@zadjii-msft
Copy link
Member

Huh, I think this got re-iterated in #7552 - I think we're gonna make it so any profile that doesn't have an icon explicitly set will automatically try to use the exe as the icon

@zadjii-msft
Copy link
Member

Looping back on this after some other work on icons.

I think we're gonna make it so any profile that doesn't have an icon explicitly set will automatically try to use the exe as the icon

This is easy enough, sorta. We can pretty easily fall back to trying to use the commandline if the icon for a profile is set to "" or null.

This kinda leads to a pair of discussion questions:

  • Do we leave the hardcoded default icon "\uE756" (the segoe fluent "Command prompt" icon)? Or do we change it to null, so that profiles will all automatically get this behavior?
    • I'm leaning towards "keep it the segoe icon".
    • We could do something weird where like, profiles from the SUI get the segoe icon by default, but then a bunch of profiles will all have a weird "icon": "\uE756" in them...
  • The only way to hide the icon becomes "icon": " ". That's weird.
  • We probably want to add a checkbox to the SUI for "Use commandline as icon", to disable the textbox.

We'll sort this out in review.

@zadjii-msft
Copy link
Member

I wonder if it would be acceptable to:

  • leave the hardcoded default as is
  • automatically use the commandline if no layers replaced it - i.e., the user didn't set the icon, AND no fragments set the icon.
  • If the user sets it to "\uE756", we'll use "\uE756".
  • If the user sets it to null, we'll hide it.

Not sure if that's something we can officially figure out

@microsoft-github-policy-service microsoft-github-policy-service bot added In-PR This issue has a related PR labels Aug 17, 2023
@zadjii-msft zadjii-msft modified the milestones: Backlog, Terminal v1.21 Feb 8, 2024
zadjii-msft added a commit that referenced this issue Feb 26, 2024
…mmandline (#15843)

Basically, title. If you null out the icon, we'll automatically try to
use the `commandline` as an icon (because we can now). We'll even be
smart about it - `cmd.exe /k echo wassup` will still just use the ico of
`cmd.exe`.

This doesn't work for `ubuntu.exe` (et. al), because that commandline is
technically a reparse point, that doesn't actually have an icon
associated with it.
Closes #705

`"none"` becomes our sentinel value for "no icon". 

This will also use the same `NormalizeCommandLine` we use for
commandline matching for finding the full path to the exe.
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Work-Item It's being tracked by an actual work item internally. (to be removed soon)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants