microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
94.21k stars 8.16k forks source link

Add a key binding to set or toggle acrylic #2531

Open moonunit9 opened 4 years ago

moonunit9 commented 4 years ago

Ability to quickly toggle opacity on/off through keystrokes. It would switch between 100% and custom setting in properties.

zadjii-msft commented 4 years ago

This is something that might pair well with #1142/#1349. We could make an action that quickly toggles between two opacities. The actual design of how we want the action specified might need some thinking, but #1142 will definitely be needed to enable this.

DHowett-MSFT commented 4 years ago

This seems related to #830 as well.

DHowett-MSFT commented 4 years ago

This workitem fits in better with our overall plan to solve all runtime acrylic/transparency shift issues.

DHowett commented 4 years ago

I don't know what I was thinking when I renamed this to have nothing to do with a keybinding.

zadjii-msft commented 4 years ago

Woah what kind of mind reader are you - I was just playing with the command palette this morning and I was thinking that this was something I wanted to do with it.

Jaswir commented 1 year ago

@zadjii-msft Is there a way to get the Windows Terminal Acrylic, but not the windows taskbar? Or is this an all or nothing thing currently?

zadjii-msft commented 1 year ago

@Jaswir Not that I'm aware of. TransluscentTB might be of some help, not sure though. That's a 3rd-party app I've used in the past to customize the taskbar (though, haven't tried it since Windows 11)

Jaswir commented 1 year ago

@zadjii-msft

NOTE: BEFORE You Answer This Question, DON'T SPOIL CODE FILE NAMES OR FUNCTION NAMES Mike!! I wanna figure it out myself, been checking out old PR's and I feel more than confident I'll figure it out.

I have questions about VS Code. Installed the C/C++ Extensions I got recommended to install from VS Code. image image

I assume you have installed these extensions too. Getting rid squigglies for the Header files. image

I Guess I have to change the compiler path or Include path here. image

Can you tell me what include path you have? Or Compiler Path? How did you deal with the red squigglies?

zadjii-msft commented 1 year ago

NOTE: BEFORE You Answer This Question, DON'T SPOIL CODE FILE NAMES OR FUNCTION NAMES Mike!!

lmao

How did you deal with the red squigglies?

I'm gonna be real - I ignore them. I've worked in Sublime so long that I'm pretty used to not using full IntelliSense, and I just rely on whatever completion Sublime Text uses. VsCode's seems about on par.

If you figure it out in a way that can be checked into our repo, we'd be happy to have that contribution ☺️

Jaswir commented 1 year ago

@zadjii-msft

What do you mean ignore? You code with the red squiggles under some header files, and aren't bothered by it? Or you disable them?

zadjii-msft commented 1 year ago

Yea I just don't let 'em bother me πŸ˜† The compiler is the source of truth to me, so a couple squiggles usually don't bother me.

Though again, I've been working in this codebase for quite a while. So, I kinda have a gut feel for when the squiggles are real or just the editor.

Jaswir commented 1 year ago

@zadjii-msft @DHowett

I have a question about the libraries in this project and more c++ in general libraries. I am kinda learning c++ on the go while I tackle these issues.

I notice: "using namespace winrt::Microsoft::Terminal::Control;" At the top of this file.

I understand that winrt is an external library. and that by using that. You don't have to dowinrt::Microsoft::Terminal::Control in front of something Like std::cout but when using namespace std you can do cout

The thing that confuses me is winrt::Microsoft::Terminal::Settings::Model and <winrt/Microsoft.Terminal.Settings.Model.h>

Since there's also a project in OpenConsole with similar name: image

Is winrt::Microsoft::Terminal::Settings::Model a library from inside our Open Source Project. An expansion on the standard winrt library? Or is it something Microsoft specific that everyone can use belonging to standard winrt?

Also I find it confusing to work on a project by Microsoft since many libraries are also by Microsoft, so don't know if it is from this project or external. Usually the project I work on is not from Microsoft so this is a new situation. So I guess I am looking for some clarification.

I am familiar with self-made libraries in C# and then using them so image Would be used inside another project with using MathLibrary; But that's without headers <winrt/Microsoft.Terminal.Settings.Model.h> Seems like a selfmade library? Expansion on standard winrt. In c++ are selfmade libraries defined as projectname + .h?

I tried using #include <winrt/windows.foundation.h> in a testing garden (that's what I call standard newly created (cpp) projects where I just test out smaller versions of puzzles I encounter in bigger projects like this" And that works but winrt/Microsoft doesn't seem to be possible

Jaswir commented 1 year ago

@zadjii-msft @DHowett @carlos-zamora @PankajBhojwani @lhecker @ianjoneill

Yes I strive to become like that. Not getting bothered by it and would really love that.

Yea I just don't let 'em bother me πŸ˜† The compiler is the source of truth to me, so a couple squiggles usually don't bother me.

Though again, I've been working in this codebase for quite a while. So, I kinda have a gut feel for when the squiggles are real or just the editor.

But I have not been working in this codebase for a while. And have no gut feeling for when squiggles are real or just the editor. And am learning cpp as I go while tackling these issues. Did 1 project with cpp and Open CV at Utrecht University in the Netherlands but that's it. Mostly did C# there. I was wondering if you can point me to someone that might give me a tip or some help with it. I'd rather not have to spend extra time figuring out the VS Code editor and instead spend it on tackling the issue. So if someone could give some advice/tips with that I would appreciate it.

I understand you folks are busy with Microsoft Build, This one can wait I work with VS for now.

zadjii-msft commented 1 year ago

Is winrt::Microsoft::Terminal::Settings::Model a library from inside our Open Source Project

Ah yes. It is. There's some... more complicated interaction going on here, so I'm gonna try and keep this as straightforward as possible. Some of this will be an oversimplification.

Basically, large parts of the Terminal codebase uses something called C++/WinRT. This is a handy toolchain for building C++ code that interacts with WinRT APIs. Anything you see in the winrt:: namespace is a WinRT type.

WinRT is special, it lets you write code in one language, expose it as a "projection" in the form of a .idl file, and then consume it from another language[^1]. So you can declare language-agnostic interfaces, classes, APIs, and just consume those APIs however you like.

In the case of C++/WinRT: We've got lots of libraries that implement lots of WinRT types. Those libraries are pretty much all named something like Microsoft.Terminal.*[^2], just out of convention. When those libraries (.dll's) are built, they produce a .winmd file, which is basically a compilation of everything in the .idl files for that library. When you're consuming APIs from that dll, we reference that project's winmd, and the C++WinRT tooling automatically generates a winrt/Microsoft.Terminal.Whatever.h to include.

You can then use those types by including that headed and instantiating something like auto foo = winrt::Microsoft::Terminal::Whatever::Foo(). That'll do some internal magic to create an instance of the winrt::Microsoft::Terminal::Whatever::implementation::Foo class, but give it back to the caller in a way such that the caller can user it however the idl defined.

It's about at this point where I'd start getting off into the weeds of "projected types" vs "implementation types", but I think that might be getting too far into the weeds.


tl;dr: Yea, the Terminal solution is composed of a bunch of different projects that each produce self-contained libraries. The winrt/Microsoft.Terminal.Whatever.h headers get generated from those libraries. After including that header, you can use types from that project like winrt::Microsoft::Terminal::Whatever::SomeClass. Everything that's Microsoft.Terminal.* is something from inside this repo. Everything that's under winrt:: is a WinRT API / Class


Please feel free to ask more questions if you have them. C++WinRT is a bit complicated at times - I'm still learning some of the ins-and-outs here πŸ˜„

[^1]: It's certainly possible, but very complicated, and not something we do in the Terminal codebase. [^2]: Except TerminalApp, which can't be in a nested namespace, because of a XAML bug.

Jaswir commented 1 year ago

@zadjii-msft

Tried setting up your setup (without teams and spotifiy, only have 2 monitors, so only left and middle one): image

Would like to have a setup with VSCode and VS where I can write C++ code in VSCode without any red or blue squigglies that don't belong. And only use VS for Debugging. Ignoring the squiggles is no option for me.

But VSCode is driving me crazy with the errors. I fixed some of include errors by adding some paths:

 "includePath": [
                "${workspaceFolder}/**",
                "C:\\terminal\\packages\\Microsoft.Windows.ImplementationLibrary.1.0.220201.1\\include",
                "C:\\Program Files (x86)\\Windows Kits\\10\\Include",
                "C:\\terminal\\src\\cascadia\\TerminalSettingsModel\\Generated Files" (This one doesn't seem to work well)
]

It's like everytime I get rid of 1 red or blue squiggly another pops up >.< image image

Some folders aren't showing up in VS Code folders that contain the missing libraries. terminal\packages C:\terminal\src\cascadia\TerminalApp\Generated Files

other folders (not containing libraries): obj bin

Help Mike? Perhaps figure out why these folders aren't showing up in VSCode? The way things are now I am giving up and going to just disable all squigglies and use VS and VSCode just for accessing files outside of the Solution.

Jaswir commented 1 year ago

@zadjii-msft

WinRT is special, it lets you write code in one language, expose it as a "projection" in the form of a .idl file, and then consume it from another language1. So you can declare language-agnostic interfaces, classes, APIs, and just consume those APIs however you like.

Yeah I noticed there is C++, C and C# all in one codebase. Does this work out of the box since they are all Microsoft created (not sure about C) . Or does WinRT Play a part here with consuming from another language thing you mentioned?

Jaswir commented 1 year ago

the C++WinRT tooling automatically generates a winrt/Microsoft.Terminal.Whatever.h to include.

That Self Generation part is tricky about this project. Thanks for the tip.

zadjii-msft commented 1 year ago

okay so this is just me messing around on the weekend. I can look more on Tuesday.

Clearly, I don't understand how vscode config works, cause this doesn't seem scalable:

{
    "configurations": [
        {
            "name": "Win32",
            "includePath": [
                "${workspaceFolder}/**",
                "${workspaceFolder}/packages/Microsoft.Taef.10.60.210621002/build/Include/**",
                "${workspaceFolder}/packages/Microsoft.Windows.ImplementationLibrary.1.0.220201.1/include/**",
                "${workspaceFolder}/src/cascadia/TerminalSettingsModel/Generated Files",
                "${workspaceFolder}/src/cascadia/TerminalApp/Generated Files"
                // It was at this point I realized this wouldn't work, as I'd have to add each manually?? That's not right.
            ],
            "defines": [
                "_DEBUG",
                "UNICODE",
                "_UNICODE"
            ],
            "windowsSdkVersion": "10.0.22621.0",
            "compilerPath": "cl.exe",
            "cStandard": "c17",
            "cppStandard": "c++20",
            "intelliSenseMode": "windows-msvc-x64",
            "browse": {
                "path": [
                    "${workspaceFolder}",
                    "${workspaceFolder}/packages/Microsoft.Taef.10.60.210621002/build/Include",
                    "${workspaceFolder}/packages/Microsoft.Windows.ImplementationLibrary.1.0.220201.1/include"
                ]
            }
        }
    ],
    "version": 4
}

It's like, it's ignoring all the paths that have autogenerated things, or are in our gitignore... hmmm

Jaswir commented 1 year ago

@zadjii-msft

Also since you're on Paternity leave who can I ask questions in your stead? Btw congrats.

Have been looking at this problem for a while and while I do move forward as I put more time in it and learn more C++ and get a much better understanding of how the entire codebase works. Like I am starting to understand how the Generated Files are working with C++/Winrt (how changes in the idl, create some other files) and can make small adaptions in smaller new imagetest projects.

Honestly if you'd pointed me here: https://learn.microsoft.com/en-us/windows/uwp/cpp-and-winrt-apis/intro-to-using-cpp-with-winrt That would've helped much more/faster.

But I feel like it can take me quite an amount of hours before I finish the issue this way.

What would make one a better Engineer you think? Keep going at it and get a much deeper understanding of the codebase and C++? Even though I'd get a much deeper understanding there's still a lot to the codebase I won't get from this issue I think and for an Engineer trying to fully grasp every codebase would take a lot of extra time leading to less issues solved per month. But having a much deeper understanding would help solve issues faster in the long run.

Or ask more questions and aim for better performance in terms of issues solved per month?

I watched this video: https://www.youtube.com/watch?v=UHyX9zBluMw&ab_channel=ArmanKhondker Where at 4.11 The guy says "If I can't unblock myself in 1 hour, I reach out for help". What is it like at Microsoft? Do engineers ask for help fast, after 1 hour being blocked?

Jaswir commented 1 year ago

@DHowett

Since Mike is on Paternity leave who can I ask questions in his stead?

Jaswir commented 1 year ago

@zadjii-msft

I'm getting stuck from multiple sides I think it's about time I start asking questions. So let me tell you what I got so far, where I'm getting stuck. I think I found a majority of the places I'll need to make changes.

What I got so far: Figured I needed to add ToggleOpacity to the command palette. Added image to terminal\src\cascadia\TerminalSettingsModel\Resources\en-US\Resources.resw

Was able to then substitute that with ExportBufferCommandKey Variants: image

So then I tried a couple angles to tackle the problem. And when writing this, I actually got even further. First angle: Add _HandleToggleOpacity in AppActionHandlers.cpp Noticed _HandleAdjustOpacity does similar functionality to HandleToggleOpacity Tried to run _HandleAdjustOpacity when __HandleExportBuffer is run. Figured I could bypass argument checking and just fill in the parameters.

image

So far I got this: what_i_got_so_far

Where I got stuck Then when actually trying to code it all neatly with _HandleToggleOpacity, ToggleOpacityArgs and Shortcut commands, things got really overwhelming.

Have been looking everywhere for a file where specific shorcuts are stored and can be adapted. Only thing I could find C:\terminal\src\cascadia\TerminalSettingsModel\defaults.json

But on top of the file it says: image

So then I spend hours looking for something better. But then when I do make a change it IS NOT ignored?? And apparently this file is not auto-generated cause when I delete it, clean the solution and rebuild it doesn't get recreated. So??? Do I have to make those shortcut storage changes here? Or someplace else? And if so where?

Oh got this now: what_i_got_so_far_command_hijack

Just hijacked: _HandleOpenSystemMenu this time

Will continue writing later about where I got stuck

DHowett commented 1 year ago

@DHowett

Since Mike is on Paternity leave who can I ask questions in his stead?

I'd be happy to try and answer questions! Since the weekend is coming up, I can't promise that we'll answer very quickly :smile:

I'm so sorry about that defaults.json thing. We put that comment at the top in our source code so that when it gets put into the final application (defaults.json inside the MSIX file!) it says that. It's to tell users that it can't be edited... but since you're looking at the copy that lives in the source code you absolutely can edit it.

Shortcuts are loaded from there by default, that's how it knows all the ones that exist.

Doing it with ToggleOpacity will require adding more handlers to "ActionArgs" and "ActionAndArgs" (silly names), which implement most of the automatic stuff that lets you write code for an action. It has been a while since I added one myself. I will have to do some research if that is not helpful enough.

Jaswir commented 1 year ago

@DHowett I see, thanks for clearing things up about shortcuts. Silly question, what is a MSIX file?

Jaswir commented 1 year ago

@moonunit9 @zadjii-msft @DHowett

About the feature request: "Ability to quickly toggle opacity on/off through keystrokes. It would switch between 100% and custom setting in properties."

Where would you like this setting to be? in settings.json? image

Like this for example: image

Or here?, like this:

image

Or both?

Jaswir commented 1 year ago

@DHowett When will Mike be back?

Jaswir commented 1 year ago

@zadjii-msft @DHowett

I'm so sorry about that defaults.json thing. We put that comment at the top in our source code so that when it gets put into the final application (defaults.json inside the MSIX file!) it says that. It's to tell users that it can't be edited... but since you're looking at the copy that lives in the source code you absolutely can edit it.

Shortcuts are loaded from there by default, that's how it knows all the ones that exist.

I suggest also adapting the comments there to make it clear the comment is aimed at users, not developers. Maybe it could save others hours of time looking for something better. I can immediately apply this while solving this issue.

// THIS IS AN AUTO-GENERATED FILE! Changes to this file will be ignored.

==>

Suggestion:

//TO USERS: THIS IS AN AUTO-GENERATED FILE! Changes to this file will be ignored. //TO DEVELOPERS OF WINDOWS TERMINAL: YOU CAN ABSOLUTELY EDIT THIS FILE, CHANGES WILL BE APPLIED //SHORTCUTS ARE LOADED FROM HERE BY DEFAULTS, THAT'S HOW IT KNOWS ALL THE ONES THAT EXIST

Jaswir commented 1 year ago

@DHowett

terminal\doc\cascadia\profiles.schema.json Looked at this file also for a while, thinking it might have some use for the solution. This file, seems to have relevant information, but when I delete everything, clean and rebuild nothing breaks? I don't understand this files purpose ... What does it do?

Jaswir commented 1 year ago

@DHowett

Blocked on this problem for a couple hours now. Trying to add ToggleOpacity inside of Command palette as a separate command and connect that to _HandleToggleOpacity in AppActionHandlers.cpp, but it doesn't appear in the command palette when I press ctrl+shift+p. I was able to get _HandleToggleOpacity in AppActionHandlers.cpp.

I added { ShortcutAction::ToggleOpacity, RS_(L"ToggleOpacityCommandKey") } in ActionAndArgs.cpp -> GenerateName and ofcourse on top of file : static constexpr std::string_view ToggleOpacityKey{ "toggleOpacity" };

and ON_ALL_ACTIONS(ToggleOpacity) To AllShortcutActions.h -> ALL_SHORTCUT_ACTIONS macro

Wondering if you could give me a tip / clue to unblock me? I tried ctrl+shift+f in VS Code looking for OpenSystem, to see whether I missed something. But not getting much wiser from it. c++/winrt with it's .idl file magic + macro's automatically creating stuff all over the place. Can make things tricky.

DHowett commented 1 year ago

Excellent questions! In order...

what is a MSIX file?

Oh, sorry. That's the installer for Terminal. It is a "packaged application".

Where would you like this setting to be?

It should show up in Actions, because it is an action that the user can assign to a keyboard key. That means in the JSON file it would show up in the "actions" section (either defaults.json or settings.json). It would also show up in the "actions" section in the settings UI!

When will Mike be back?

In a few weeks!

...adapting the comments in defaults.json...

Not a bad idea. However, adding a message for the developers of Terminal to that file mean that all 170 million users also need to have a copy of the message that's for developers only. We can work on making it clearer, for sure.

schema.json

That is the file that tells Visual Studio Code what keys and values are "legal" when you are editing settings.json! It says "a profile can have these things inside it" and "these are the actions that are legal after the 'action' key".

Adding ToggleOpacity inside the Command Palette

Those three places - ActionAndArgs GenerateName, the key name, and ON_ALL_ACTIONS are the right places to put it!

I believe the last place you need to look is defaults.json, in the "actions" section. It might be like these ones:

https://github.com/microsoft/terminal/blob/8aefc7a697ec155258d2fc0d84ea3466aaa0754e/src/cascadia/TerminalSettingsModel/defaults.json#L374-L376

If you have hooked it up correctly, that is the last "piece" you need since this file tells Terminal which actions should be created by default.[^1]

[^1]: This is not my favorite part of our code, because it does mean that when you add a new action it needs to happen in approximately four separate places. ☹️

Jaswir commented 1 year ago

@DHowett

Those three places - ActionAndArgs GenerateName, the key name, and ON_ALL_ACTIONS are the right places to put it!

I believe the last place you need to look is defaults.json, in the "actions" section. It might be like these ones:

Already had it in the file, but it wasn't working, but now it is. Ah well whatever , it works now I guess :)

I believe the last place you need to look is defaults.json, in the "actions" section. It might be like these ones:

Doing it with ToggleOpacity will require adding more handlers to "ActionArgs" and "ActionAndArgs" (silly names), which implement most of the automatic stuff that lets you write code for an action. It has been a while since I added one myself. I will have to do some research if that is not helpful enough.

Honestly I don't feel comfortable being pointed to the precise files. I am worried about my development as an Engineer.

A big part of me helping out in this Open Source project is wanting to become a great Engineer like you. Getting pointed to the filenames conflicts with that.

Basically the give a man a fish idea: β€œIf you give a man a fish, you feed him for a day. If you teach a man to fish, you feed him for a lifetime.” A good Engineer is able to think for themselves, find the specific files themselves right? In real life software engineering situations more often than not, there won't be someone pointing me to the filenames right? By having you point me to them, you are effectively doing the thinking for me. And so without that I'd be lost right?

At the same time I have been kinda running in circles with this one for a while. And having some confirmation with the filenames is nice.

Jaswir commented 1 year ago

@DHowett

It should show up in Actions, because it is an action that the user can assign to a keyboard key. That means in the JSON file it would show up in the "actions" section (either defaults.json or settings.json). It would also show up in the "actions" section in the settings UI!

It should also show up in settings.json. So should have an entry in schema.json. And it should also be available in the settings.json right, like that, so you can set a value there and then make it toggle to that opacity. image

PS: don't point to filenames If it needs to be stored there unless it would make me a great engineer like you maybe I am overthinking this. Do Engineers at Microsoft also get pointed to filenames???

That is the file that tells Visual Studio Code what keys and values are "legal" when you are editing settings.json! It says "a profile can have these things inside it" and "these are the actions that are legal after the 'action' key".

I removed Schema.json Cleaned project, rebuild. But still all the properties are there to choose from in visual studio code??? I expect them to be gone, based on what you say.

schema-json-not-doing-stuff-yet

Jaswir commented 1 year ago

@DHowett This is what I got so far, almost done. Only need to use toggleOpacity instead of backgroundImageOpacity. That's where I am blocked now.

what_I_got_so_far_2

It's not the same without Mike. For me what makes programming work fun is adding value to the team and being appreciated for that and social communication. This energizes me. Without @zadjii-msft I think Ima take a break too after finishing this one since I am so close. Till Mike is back. Hence why I can't make projects on my own, and seek collabs like open source. Let me know when you're back @zadjii-msft

Jaswir commented 1 year ago

@DHowett

Maybe a tip/hint here?? Trying to make a change to the properties by changing BackgroundImageOpacity to BackgroundImageOpacity22. Changed BackgroundImageOpacity any place I could find it in the code to BackgroundImageOpacity22, but didn't work. Get 100+ errors. I noticed some of the macro's do a lot of work under the hood that you can't really look for with ctrl+shift+f in VS-Code. It could be I missed a Macro? Or I changed all the names in all files at once and somehow something didn't get generated because of doing this?

I changed this to 22 in Appearances.idl:
OBSERVABLE_PROJECTED_APPEARANCE_SETTING(Double, BackgroundImageOpacity22);

And this in Appearances.h:

OBSERVABLE_PROJECTED_SETTING(_appearance, BackgroundImageOpacity22);

So I expect that backgroundimageopacity22 should have been generated as observable projected settings but appearently cause it says this: image

Am I missing something?

But image

Jaswir commented 1 year ago

@zadjii-msft Hi Mike, congrats on your new child. Hope you had a nice paternity leave. Your status said on paternity leave for June, and June is over. But the status is still there. I was wondering whether you're back?

And can maybe help with above mentioned blocker? Looking forward to finish this issue. And atleast get a second issue done in an open source project this time, before giving up. Or even better, not giving up and going for more issues :D

@DHowett You know whether Mike is back yet? Or when he'll be back?

zadjii-msft commented 1 year ago

Hey, sorry for the delay. Yesterday was a holiday so it was a bit of an extra long weekend at the end of the vacation ☺️ Still trudging through a large pile of emails, but lemme see if I can't unblock you.

From just skimming this thread, it looks like you've got the action set up to plumb into the TermControl / ControlCore layer. That's great! I think you might be a bit off track with the OBSERVABLE_PROJECTED_SETTING bit. Try looking at this instead:

https://github.com/microsoft/terminal/blob/94e6b91c7892b9e2ec8244ab7990ecf50679b083/src/cascadia/TerminalControl/ControlCore.h#L234-L235

Those two are special - they're control properties that could be changable at runtime[^1]. There's already an action for setting Opacity at runtime[^2]. We were prepared though, and knew we wanted to do a similar thing with acrylic, so we pre-emptively made that a RUNTIME_SETTING so it could also be changed in a similar way. I'd start there.

PS: don't point to filenames If it needs to be stored there unless it would make me a great engineer like you maybe I am overthinking this. Do Engineers at Microsoft also get pointed to filenames???

We absofuckinglutely do, all the time. Dustin linked me a file path literally this morning. It's 1000x better to ask for help and quickly get pointed in the right direction, then spin your wheels for a couple days. There's literally no shame in admitting you don't know something, and asking for help.

[^1]: I believe added in #11619 [^2]: added in #12092

Jaswir commented 1 year ago

@zadjii-msft

We were prepared though, and knew we wanted to do a similar thing with acrylic, so we pre-emptively made that a RUNTIME_SETTING so it could also be changed in a similar way. I'd start there.

Wait I am confused now. Thought the assignment of this issue was to make a toggle for opacity besides #12092 as mentioned by @moonunit9 .

Ability to quickly toggle opacity on/off through keystrokes. It would switch between 100% and custom setting in properties

The ones in #12092 are specific to 0,25,50,75. But no in-between. This one would switch to a custom value between 0 and 100 , like 58. Hence I was looking for a way to add a toggleOpacity property to the settings, besides BackgroundImageOpacity.

But the title of the issue is key binding to set or toggle acrylic. And from what you're saying, the goal is to make it toggle acrylic on and off instead? You want me to make something like this?

One sec, I'm working on a gif.

Jaswir commented 1 year ago

Toggle_Acrylic

@zadjii-msft

Here is something I could get working in code. Is this the idea? Toggling Acrylic? If so, I don't know why_runtimeUseAcrylic = !_runtimeUseAcrylic doesn't work. Right now I only can do _runtimeUseAcrylic = true or _runtimeUseAcrylic = false, which does work.

Why doesn't _runtimeUseAcrylic = !_runtimeUseAcrylic work?

zadjii-msft commented 12 months ago

Wait I am confused now. Thought the assignment of this issue was to make a toggle for opacity besides

Oh, huh. Good observation. Seems like over the course of the history of this work item, we retitled it a bunch of times. That caused us to actually entirely lose the meaning of the original request... huh. Well now that's on us for being confusing.

I personally don't really feel great about a "Toggle opacity" action. My main concern is what opacity does it toggle to (from opaque)? Like, do we need to always just pick and opacity that the action goes to? That feels weird. Say we pick 30% as our arbitrary bound. Then a user could toggle from 50% -> 100% (off) -> 30%, which is probably not what they expected.

We could maybe do the action as "Toggle opacity, opacity:{}" and leave opacity as an argument. So something like:

{ "command": { "action": "toggleOpacity", "opacity": 30 } },

when you do that action, it'll either:

But then, that precipitates the question of should this be a parameter to adjustOpacity - like,

{ "command": { "action": "adjustOpacity", "opacity": 30, "toggle": true } },

idk. I honestly just think the existing "Set opacity" actions cover the original scenario well enough...


I still think there's value in a toggleAcrylic action, which is really what I wanted from this thread (and was expecting from the title). What you've got in the above gif looks like basically what I was thinking of ☺️


Why doesn't _runtimeUseAcrylic = !_runtimeUseAcrylic work?

Ah, that's because _runtimeUseAcrylic is technically, secretly, a std::optional<bool>. So it starts as nullopt, and presumably optional::operator!(nullopt) -> nullopt.

Jaswir commented 11 months ago

@zadjii-msft

This is what I got now: toggle acrylic v1

Jaswir commented 11 months ago

@zadjii-msft

But it's kinda buggy, look what happens when I change opacity with mouse wheel: buggy toggle acrylic

It's going out of Acrylic. Not sure why ScrollWheel Bug is happening. My guess is that because it's runtime Acrylic instead of the saved one in settings.json.

Jaswir commented 11 months ago

@zadjii-msft Yeah it's this line : // Manually turn off acrylic if they turn off transparency. _runtimeUseAcrylic = newOpacity < 1.0 && _settings->UseAcrylic();

From ControlCore::_setOpacity

When:

 "defaults": 
        {
            "useAcrylic": false
        },

The acrylic get's set to off everytime you change opacity with scrollwheel

runtime acrylic is set to off everytime opacity is changed when "useAcrylic": false in the settings.json With the addition of Toggle Acrylic this makes no sense anymore hence It seems right to change it to _runtimeUseAcrylic = newOpacity < 1.0;

Uhm I meant:

   // Manually turn off acrylic if they turn off transparency.
        if (newOpacity == 1.0)
        {
            _runtimeUseAcrylic = false;
        }
Jaswir commented 11 months ago

Only thing I do still wonder about.

Currently I use this:

        auto eventArgs = winrt::make_self<TransparencyChangedEventArgs>(0.0);
        _TransparencyChangedHandlers(*this, *eventArgs);

Inside ToggleAcrylic in ControlCore.cpp , I am thinking of making a new ChangedEventArgs for Acrylic , since I use an arbitrary value of 0.0 to get this working. Then again, intuitively I feel better about using existing code if it can be reused. Instead of adding new code.

Jaswir commented 11 months ago

Anyway there's a PR

Jaswir commented 11 months ago

@zadjii-msft As for the next issue. This one seems like fun to me : https://github.com/microsoft/terminal/issues/11092 And you said this one first: 7158 So I guess this adventure pretty much. Assuming AbdullahAlmanei isn't working on it.

I thought doing this issue would be a good prep. And have been eyeballing those for a while to be honest.

But I would feel uncomfortable talking in the chat of 7158 cause lots of people. I was wondering if I can also email you or contact you somewhere more private, that would feel more comfortable. Do you respond similar to your email as in the chat here? Like here you respond quite fast. I saw your email somewhere, but can't find it anymore. What was it again? I guess mikegriese@microsoft.com ?

Btw I like to do issues that lots of people appreciate. In the past there was a period of my life where I made innovative web games. And mostly implemented the things most people asked for as they added most value. Here is something I made: https://www.kongregate.com/games/FeatherHatGames/toughgrowth? ref=blog.kongregate.com Or this when the other one doesn't work (haven't updated it with the webbrowser updates, but I think they update stuff themselves here to make it work) https://armorgames.com/tough-growth-game/18348

However doing stuff like that with a team is new for me.

Jaswir commented 11 months ago

@zadjii-msft

I started a bit on it and this is what I got. start_plumbing

Jaswir commented 11 months ago

@zadjii-msft

As in waiting for a more optimal comfortable place to talk with you. I guess I talk here for now.. Trying to get a clearer picture of what to make. Have a question.

  1. We want a boolean in defaults like this right?

image

  1. Also will the changes mentioned in the walkthrough in 7158 Add something here? No right? That is done in a seperate Xaml file or no?

image

  1. I don't know why this tooltip doesn't change to "dragonpoop" or "experimental.enableUnfocusedAcrylic.showMarksOnScrollbar"

I looked for the tooltip text in the project and changed doc\cascadia\profiles.schema.json image

As well as the text in this line X(bool, ShowMarks, "experimental.enableUnfocusedAcrylic.showMarksOnScrollbar", false)
in MTSMSettings.h Trying to make something happen in the settings file, but nothing happens.

zadjii-msft commented 11 months ago

Hey sorry, lots of stuff to catch up on and I'm still quite distracted at the moment. Trying to get through a couple notes:

But I would feel uncomfortable talking in the chat of 7158 cause lots of people. I was wondering if I can also email you or contact you somewhere more private, that would feel more comfortable. Do you respond similar to your email as in the chat here?

That's totally reasonable - that's one of the most trafficked open issues at this point. Though, I'd probably stick to GitHub - my mail filters are notoriously aggressive and unreliable. Plus, I'm just better at keeping up to date on here anyways.

We want a boolean in defaults like this right?

Er, probably not in the profiles.defaults in the json itself. A hard-coded default is probably fine. I think those are in either MTSMSettings.h or TerminalSettings.h or ControlProperties.h... one of those. Might be all of them 😝

{{where do we put this in the XAML}}

I'd probably suggest... GlobalAppearance.xaml? since this would be a global appearance-related setting?

I actually think we may want to skip the XAML on this one for now. We've been having some discussions about compatibility settings, global settings, stuff like this, and it might be _more- confusing to have you do something here. We can always add it in post when we decide where to put it.

Schema changes aren't taking effect!

Oh jeez editing the schema is always a pain. You need to change

    "$schema": "https://aka.ms/terminal-profiles-schema",

at the top of your settings.json file to point at the profiles.schema.json on disk that you're making changes to, then reload VSCode. But I always forget the precise syntax for that and I'm away from the PC that has the right one. Something file://path-to-schema but again, I forget it exactly.

Jaswir commented 11 months ago

at the top of your settings.json file to point at the profiles.schema.json on disk that you're making changes to, then reload VSCode. But I always forget the precise syntax for that and I'm away from the PC that has the right one. Something file://path-to-schema but again, I forget it exactly.

It'sfile:/// , thanks for the tip :)

@zadjii-msft When would it suit you to review 2531?

Jaswir commented 11 months ago

Er, probably not in the profiles.defaults in the json itself. A hard-coded default is probably fine. I think those are in either MTSMSettings.h or TerminalSettings.h or ControlProperties.h... one of those. Might be all of them 😝

Not in profiles I get that. I have it in the Globals now, so it works like this: global_settings_boolean

I don't know what you mean with hard-coded default. What I understand the boolean value is set globally in settings.json like in the gif. The code part (MTSMSettings.h or TerminalSettings.h or ControlProperties.h) then connects to that and from there I can connect it to TermControl to either use in-app-acrylic or not.

For now it can be hard coded inside settings.json. But no representative yet in Xaml, so nothing will appear here: image

As of yet.

Is that what you mean with hard-coded? (not appearing in the Xaml, but only in global settings) Making sure I am implementing the right thing. Still have to connect the code to the settings.json though, but this is what I got now.

Jaswir commented 11 months ago

Hmm, would expect that there would be some kind of list outside of settings.json that checks whether jsonKey's entered at MTSMSettings.h are valid. The valid properties would be stores in this list, or maybe double check with the profiles.schema.json either on disk or here: https://aka.ms/terminal-profiles-schema".

And this check thing might be called controlproperties at it controls whether properties exist.