Add support for OSC777 (Send Notification)#20012
Add support for OSC777 (Send Notification)#20012carlos-zamora wants to merge 22 commits intomainfrom
Conversation
4357c17 to
aeb531f
Compare
2f5763e to
34b985d
Compare
This comment has been minimized.
This comment has been minimized.
## Summary of the Pull Request Targets #20010 Manually assign an AUMID to our process when we're running unpackaged. Main difference from #19937 is what AUMID we use. Before, it was per branding, but the `WindowEmperor` already appends an exe path hash for unpackaged instances to prevent crosstalk. Here, we're just using the same pattern: `Microsoft.WindowsTerminal.<hash>`. Heavily based on #19937 Co-authored by @zadjii-msft
34b985d to
7ea4f2f
Compare
This comment has been minimized.
This comment has been minimized.
7a378e1 to
33546f1
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
5788845 to
36c361e
Compare
36c361e to
d4ff09f
Compare
There was a problem hiding this comment.
wait, this seems wrong. it's not "Activity," is it?
There was a problem hiding this comment.
Hmm... I think this one's fine, actually. It's all used here:
terminal/src/cascadia/TerminalApp/TabManagement.cpp
Lines 1226 to 1266 in fcaf3da
So, if OSC777 includes a body, we use the body & title from the sequence.
Otherwise, we fall back to these resources to say a generic "Activity in tab X".
Open to suggestions.
There was a problem hiding this comment.
qq: why does this need a public class type of its own? It's an implementation detail used only inside TerminalApp to talk to TerminalApp
There was a problem hiding this comment.
Really just because we're using...
// IPaneContent.idl
event Windows.Foundation.TypedEventHandler<IPaneContent, NotificationEventArgs> NotificationRequested;
I could update it to using a delegate so that we don't have a new public class type and we just pass the payload directly instead. Deciding against that in this PR because (1) it's a different style from the other ones in this file and (2) #20014 also uses this system to pass the payload up.
That said, I'm open to doing that as a follow-up once both of these merge. I think it's worth it and it looks nicer.
To get an idea of what it would look like:
// IPaneContent.idl
delegate void NotificationRequestedHandler(String title, String body);
interface IPaneContent
{
event NotificationRequestedHandler NotificationRequested;
}
Feedback from Bug Bash (5/5)
@DHowett Got a reason why? Is it that we just don't want applications to spam the user with notifications? |
|
Updated default to Validated that this works. Can be tested using: Write-Host "$([char]27)]777;notify;Title;Message$([char]27)\" |
Summary of the Pull Request
targets #20010
This adds support for the
OSC 777 ; notify ; title ; body STsequence. This allows client applications to send a notification to the Terminal. When this notification is clicked, it summons the terminal window that sent it.Validation Steps Performed
PR Checklist
Heavily based on #19938
Co-authored by @zadjii-msft