Add `Microsoft.Terminal.Remoting.dll` #8607

zadjii-msft · 2020-12-17T20:49:00Z

Adds a Microsoft.Terminal.Remoting.dll to our solution. This DLL will
be responsible for all the Monarch/Peasant work that's been described in
#7240 & #8135.

This PR does not implement the Monarch/Peasant architecture in any
significant way. The goal of this PR is to just to establish the project
layout, and the most basic connections. This should make reviewing the
actual meat of the implementation (in a later PR) easier. It will also
give us the opportunity to include some of the basic weird things we're
doing (with CoRegisterClass) in the Terminal now, and get them
selfhosted, before building on them too much.

This PR does have windows registering the Monarch class with COM. When
windows are created, they'll as the Monarch if they should create a new
window or not. In this PR, the Monarch will always reply "yes, please
make a new window".

Similar to other projects in our solution, we're adding 3 projects here:

Microsoft.Terminal.Remoting.lib: the actual implementation, as a
static lib.
Microsoft.Terminal.Remoting.dll: The implementation linked as a DLL,
for use in WindowsTerminal.exe.
Remoting.UnitTests.dll: A unit test dll that links with the static
lib.

There are plenty of TODOs scattered about the code. Clearly, most of
this isn't implemented yet, but I do have more WIP branches. I'm using
projects/5 as my
notation for TODOs that are too small for an issue, but are part of the
whole Process Model 2.0 work.

References

Scenario: Windows Terminal 2.0 Process Model Improvements #5000 - this is the process model megathread
Spec for Windows Terminal Process Model 2.0 #7240 - The process model 2.0 spec.
Spec for Windows Terminal Window Management #8135 - the window management spec. (please review me, I have 0/3
signoffs even after the discussion we had 😢)
Add a Monarch/Peasant sample app #8171 - the Monarch/peasant sample. (please review me, I have 1/2)

PR Checklist

Closes nothing, this is just infrastructure
I work here
Tests added/passed
[n/a] Requires documentation to be updated

…ompile a String[]

…then back!

…!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

build/Helix/runtests.cmd

miniksa

Marking req changes so I can see what changed since I publish these. They're not super pressing comments, though.

src/cascadia/Remoting/CommandlineArgs.cpp

src/cascadia/Remoting/Microsoft.Terminal.RemotingLib.vcxproj

src/cascadia/Remoting/WindowManager.cpp

src/cascadia/Remoting/WindowManager.h

src/cascadia/WindowsTerminal/AppHost.cpp

tools/tests.xml

zadjii-msft · 2020-12-18T13:11:01Z

@DHowett you and I discussed being worried about peasants crashing after their original monarch is closed. Turns out, we don't need to worry about that in this PR! The monarch is only ever talked to once, at startup. After that, the window totally behaves as if business as usual.

carlos-zamora

No issues with the implementation. Really you're just missing a few copyright headers haha

carlos-zamora · 2020-12-24T00:48:45Z

src/cascadia/TerminalApp/AppLogic.cpp

+            _root->ProcessStartupActions(actions, false);
+        }
+
+        return result; // TODO:MG does a return value make sense


Looks like you're not using it for now, so I guess don't return a value and change it later if/when we need it?

src/cascadia/Remoting/WindowManager.cpp

src/cascadia/Remoting/MonarchFactory.h

src/cascadia/Remoting/Monarch.idl

src/cascadia/Remoting/Monarch.h

src/cascadia/Remoting/Monarch.cpp

src/cascadia/Remoting/dll/Microsoft.Terminal.Remoting.vcxproj

DHowett · 2021-01-03T00:54:02Z

Is the new project audit-enabled? Should it be?

….dll

zadjii-msft · 2021-01-05T17:50:41Z

Is the new project audit-enabled? Should it be?

Added audit to it in the last push ✔️

DHowett

Moderately worried about the header things, but you can merge it if you plan to address it better later.

src/cascadia/Remoting/MonarchFactory.h

….dll

ghost · 2021-01-07T21:24:46Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

src/cascadia/Remoting/MonarchFactory.h

zadjii-msft · 2021-01-07T21:26:42Z

@msftbot don't merge this PR until 9pm

ghost · 2021-01-07T21:26:45Z

Hello @zadjii-msft!

I think you told me that you want to delay the approval for a certain amount of time, but I am not confident that I have understood you correctly.

Please try rephrasing your instruction to me.

zadjii-msft · 2021-01-07T21:27:09Z

@msftbot wait 4 hours to merge this PR

ghost · 2021-01-07T21:27:13Z

Hello @zadjii-msft!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I won't merge this pull request until after the UTC date Fri, 08 Jan 2021 01:27:12 GMT, which is in 4 hours

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

src/cascadia/Remoting/Monarch.h

zadjii-msft · 2021-01-07T22:42:31Z

@msftbot forget everything I just told you

ghost · 2021-01-07T22:42:35Z

Hello @zadjii-msft!

Because you've told me to reset the custom auto-merge settings, I'll use the configured settings for this repository when I'm merging this pull request.

ghost · 2021-01-28T18:02:52Z

🎉Windows Terminal Preview v1.6.10272.0 has been released which incorporates this pull request.:tada:

Handy links:

Adds a `Microsoft.Terminal.Remoting.dll` to our solution. This DLL will be responsible for all the Monarch/Peasant work that's been described in microsoft#7240 & microsoft#8135. This PR does _not_ implement the Monarch/Peasant architecture in any significant way. The goal of this PR is to just to establish the project layout, and the most basic connections. This should make reviewing the actual meat of the implementation (in a later PR) easier. It will also give us the opportunity to include some of the basic weird things we're doing (with `CoRegisterClass`) in the Terminal _now_, and get them selfhosted, before building on them too much. This PR does have windows registering the `Monarch` class with COM. When windows are created, they'll as the Monarch if they should create a new window or not. In this PR, the Monarch will always reply "yes, please make a new window". Similar to other projects in our solution, we're adding 3 projects here: * `Microsoft.Terminal.Remoting.lib`: the actual implementation, as a static lib. * `Microsoft.Terminal.Remoting.dll`: The implementation linked as a DLL, for use in `WindowsTerminal.exe`. * `Remoting.UnitTests.dll`: A unit test dll that links with the static lib. There are plenty of TODOs scattered about the code. Clearly, most of this isn't implemented yet, but I do have more WIP branches. I'm using [`projects/5`](https://github.com/microsoft/terminal/projects/5) as my notation for TODOs that are too small for an issue, but are part of the whole Process Model 2.0 work. ## References * microsoft#5000 - this is the process model megathread * microsoft#7240 - The process model 2.0 spec. * microsoft#8135 - the window management spec. (please review me, I have 0/3 signoffs even after the discussion we had 😢) * microsoft#8171 - the Monarch/peasant sample. (please review me, I have 1/2) ## PR Checklist * [x] Closes nothing, this is just infrastructure * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated

zadjii-msft added 15 commits December 16, 2020 07:59

Get all the projects created and hooked up to the sln

3bef7bb

Yank all the M/P files, this builds?!

1f52d35

Shockingly, this works, it works elevated, and it works unpackaged

5a9cdc8

This won't work, but I'm committing this becaus I finally got it to c…

36539cf

…ompile a String[]

whoop, we pass the commandline from the peasant, to the monarch, and …

27ace16

…then back!

HOLY SHIT I GOT THE COMANDLINE TO EXECUTE IN THE CURRENT WINDOW!!!!!!…

9a41647

…!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

add a note to future me

5cabcfb

This works as a unittest, but not a local test. That's batty

03bfc6e

this macro makes me feel dirty

590b9ff

LocalTests_Remoting -> UnitTests_Remoting

0579b24

finish renaming this test

a3faed6

Clean up a lot for review

9c6eac4

cleanup Peasant for review

0103331

Final cleanup for review

b4fe1bf

_final_ final cleanup for review

d08e65c

miniksa reviewed Dec 17, 2020

View reviewed changes

build/Helix/runtests.cmd Show resolved Hide resolved

miniksa requested changes Dec 17, 2020

View reviewed changes

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Dec 17, 2020

pr nits

e101efd

miniksa approved these changes Dec 18, 2020

View reviewed changes

carlos-zamora approved these changes Dec 24, 2020

View reviewed changes

DHowett reviewed Jan 3, 2021

View reviewed changes

src/cascadia/Remoting/dll/Microsoft.Terminal.Remoting.vcxproj Outdated Show resolved Hide resolved

zadjii-msft mentioned this pull request Jan 4, 2021

Multiple "Open in Windows Terminal" opes multiple Windows Terminals rather than tabs #7071

Open

zadjii-msft self-assigned this Jan 4, 2021

zadjii-msft added 3 commits January 5, 2021 11:19

Merge remote-tracking branch 'origin/main' into dev/migrie/f/remoting…

fa2df47

….dll

Enable audit mode

0f5c24f

nits

921d915

zadjii-msft removed their assignment Jan 5, 2021

DHowett reviewed Jan 6, 2021

View reviewed changes

src/cascadia/Remoting/MonarchFactory.h Outdated Show resolved Hide resolved

DHowett changed the title ~~Adds Microsoft.Terminal.Remoting.dll~~ Add Microsoft.Terminal.Remoting.dll Jan 6, 2021

zadjii-msft mentioned this pull request Jan 6, 2021

Add support for wt.exe to run commands in an existing Terminal Window #4472

Closed

zadjii-msft added 2 commits January 7, 2021 14:59

Merge remote-tracking branch 'origin/main' into dev/migrie/f/remoting…

88ffc6f

….dll

putting var in headers is bad, mkay?

be74b2e

zadjii-msft added Area-Build Issues pertaining to the build system, CI, infrastructure, meta AutoMerge Marked for automatic merge by the bot when requirements are met Product-Terminal The new Windows Terminal. labels Jan 7, 2021

DHowett reviewed Jan 7, 2021

View reviewed changes

src/cascadia/Remoting/MonarchFactory.h Show resolved Hide resolved

DHowett reviewed Jan 7, 2021

View reviewed changes

src/cascadia/Remoting/Monarch.h Outdated Show resolved Hide resolved

branding

2a7bc94

ghost merged commit 7d503a4 into main Jan 7, 2021

ghost deleted the dev/migrie/f/remoting.dll branch January 7, 2021 22:59

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Microsoft.Terminal.Remoting.dll` #8607

Add `Microsoft.Terminal.Remoting.dll` #8607

zadjii-msft commented Dec 17, 2020 •

edited by DHowett

Loading

miniksa left a comment

zadjii-msft commented Dec 18, 2020

carlos-zamora left a comment

carlos-zamora Dec 24, 2020

DHowett commented Jan 3, 2021

zadjii-msft commented Jan 5, 2021

DHowett left a comment

ghost commented Jan 7, 2021

zadjii-msft commented Jan 7, 2021

ghost commented Jan 7, 2021

zadjii-msft commented Jan 7, 2021

ghost commented Jan 7, 2021

zadjii-msft commented Jan 7, 2021

ghost commented Jan 7, 2021

ghost commented Jan 28, 2021

Add Microsoft.Terminal.Remoting.dll #8607

Add Microsoft.Terminal.Remoting.dll #8607

Conversation

zadjii-msft commented Dec 17, 2020 • edited by DHowett Loading

References

PR Checklist

miniksa left a comment

Choose a reason for hiding this comment

zadjii-msft commented Dec 18, 2020

carlos-zamora left a comment

Choose a reason for hiding this comment

carlos-zamora Dec 24, 2020

Choose a reason for hiding this comment

DHowett commented Jan 3, 2021

zadjii-msft commented Jan 5, 2021

DHowett left a comment

Choose a reason for hiding this comment

ghost commented Jan 7, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

zadjii-msft commented Jan 7, 2021

ghost commented Jan 7, 2021

zadjii-msft commented Jan 7, 2021

ghost commented Jan 7, 2021

zadjii-msft commented Jan 7, 2021

ghost commented Jan 7, 2021

ghost commented Jan 28, 2021

Add `Microsoft.Terminal.Remoting.dll` #8607

Add `Microsoft.Terminal.Remoting.dll` #8607

zadjii-msft commented Dec 17, 2020 •

edited by DHowett

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.