close
The Wayback Machine - https://web.archive.org/web/20201224035340/https://github.com/microsoft/terminal/pull/8607
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

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

Open
wants to merge 16 commits into
base: main
from

Conversation

@zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Dec 17, 2020

Summary of the Pull Request

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

  • #5000 - this is the process model megathread
  • #7240 - The process model 2.0 spec.
  • #8135 - the window management spec. (please review me, I have 0/3 signoffs even after the discussion we had 😢)
  • #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
@github-actions

This comment has been hidden.

Copy link

@github-actions github-actions bot commented on 27ace16 Dec 16, 2020

New misspellings found, please review:

  • IPeasant
  • llu
  • MULTIPLEUSE
  • REGCLS
  • tmplate
To accept these changes, run the following commands
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/5757ec679b03a4240130c3c53766c91bbc5cd6a7.txt
.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt
.github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"AAAAA Bopomofo Checkboxes CParams cso CSV DOWNSCALE GENERATEPROJECTPRIFILE hhhh Horiz Inlines INPATHROOT intersectors Kode MAKEINTRESOURCEA mousemode msixbundle psin Reserialize reso RETROII rgus sbri scanbri scol SGRXY spe sph spherefunctions tcon UDK UDKs Unfocus Viginetting wz xe xlang XWV xyw yzx "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/27ace166529367171fc81cef12aa4812e6ff8e13.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"checkboxes csv horiz inlines IPeasant llu MULTIPLEUSE REGCLS reserialize tmplate udk unfocus "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/27ace166529367171fc81cef12aa4812e6ff8e13.txt is added to your repository...'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

…!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
@github-actions

This comment has been hidden.

Copy link

@github-actions github-actions bot commented on 9a41647 Dec 17, 2020

New misspellings found, please review:

  • IPeasant
  • llu
  • MULTIPLEUSE
  • REGCLS
  • tmplate
To accept these changes, run the following commands
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/5757ec679b03a4240130c3c53766c91bbc5cd6a7.txt
.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt
.github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"AAAAA Bopomofo Checkboxes CParams cso CSV DOWNSCALE GENERATEPROJECTPRIFILE hhhh Horiz Inlines INPATHROOT intersectors Kode MAKEINTRESOURCEA mousemode msixbundle psin Reserialize reso RETROII rgus sbri scanbri scol SGRXY spe sph spherefunctions tcon UDK UDKs Unfocus Viginetting wz xe xlang XWV xyw yzx "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/9a41647ffe00d973f11478fb937221239d6ac81a.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"checkboxes csv horiz inlines IPeasant llu MULTIPLEUSE REGCLS reserialize tmplate udk unfocus "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/9a41647ffe00d973f11478fb937221239d6ac81a.txt is added to your repository...'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

This comment has been minimized.

Copy link
Collaborator

@oising oising replied Dec 17, 2020

Someone's excited :)

This comment has been minimized.

Copy link
Member Author

@zadjii-msft zadjii-msft replied Dec 17, 2020

Uh, yea I am! After months of prototyping and debate, this is finally coming together. It feels good ☺️

@github-actions

This comment has been hidden.

Copy link

@github-actions github-actions bot commented on a3faed6 Dec 17, 2020

New misspellings found, please review:

  • IPeasant
  • llu
  • MULTIPLEUSE
  • REGCLS
  • tmplate
To accept these changes, run the following commands
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/5757ec679b03a4240130c3c53766c91bbc5cd6a7.txt
.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt
.github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"AAAAA Bopomofo Checkboxes CParams cso CSV DOWNSCALE GENERATEPROJECTPRIFILE hhhh Horiz Inlines INPATHROOT intersectors Kode MAKEINTRESOURCEA mousemode msixbundle psin Remoting Reserialize reso RETROII rgus sbri scanbri scol SGRXY spe sph spherefunctions tcon UDK UDKs Unfocus Viginetting wz xe xlang XWV xyw yzx "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/a3faed6b7d82049b7f43162bfd88bd8534d7de15.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"checkboxes csv horiz inlines IPeasant llu MULTIPLEUSE REGCLS remoting reserialize tmplate udk unfocus "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/a3faed6b7d82049b7f43162bfd88bd8534d7de15.txt is added to your repository...'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

@github-actions

This comment has been hidden.

Copy link

@github-actions github-actions bot commented on b4fe1bf Dec 17, 2020

New misspellings found, please review:

  • arguements
  • IPeasant
  • MULTIPLEUSE
  • Othersize
  • REGCLS
  • themself
  • tmplate
To accept these changes, run the following commands
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/5757ec679b03a4240130c3c53766c91bbc5cd6a7.txt
.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt
.github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"AAAAA Bopomofo Checkboxes CParams cso CSV DOWNSCALE GENERATEPROJECTPRIFILE hhhh Horiz Inlines INPATHROOT intersectors Kode MAKEINTRESOURCEA mousemode msixbundle psin Remoting Reserialize reso RETROII rgus sbri scanbri scol SGRXY spe sph spherefunctions tcon UDK UDKs Unfocus Viginetting wz xe xlang XWV xyw yzx "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/b4fe1bffbfed2fecad8f8c61d6a1ee0c3f01efbc.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"arguements checkboxes csv horiz inlines IPeasant MULTIPLEUSE Othersize REGCLS remoting reserialize themself tmplate udk unfocus "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/b4fe1bffbfed2fecad8f8c61d6a1ee0c3f01efbc.txt is added to your repository...'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

@github-actions

This comment has been minimized.

Copy link

@github-actions github-actions bot commented on d08e65c Dec 17, 2020

New misspellings found, please review:

  • arguements
  • MULTIPLEUSE
  • Othersize
  • REGCLS
  • themself
To accept these changes, run the following commands
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/5757ec679b03a4240130c3c53766c91bbc5cd6a7.txt
.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt
.github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"AAAAA Bopomofo Checkboxes CParams cso CSV DOWNSCALE GENERATEPROJECTPRIFILE hhhh Horiz Inlines INPATHROOT intersectors Kode MAKEINTRESOURCEA mousemode msixbundle psin Remoting Reserialize reso RETROII rgus sbri scanbri scol SGRXY spe sph spherefunctions tcon UDK UDKs Unfocus Viginetting wz xe xlang XWV xyw yzx "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/d08e65ce032fba14c24665907336986f8ec1ac7a.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"arguements checkboxes csv horiz inlines MULTIPLEUSE Othersize REGCLS remoting reserialize themself udk unfocus "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/d08e65ce032fba14c24665907336986f8ec1ac7a.txt is added to your repository...'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

Copy link
Member

@miniksa miniksa left a comment

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

src/cascadia/Remoting/WindowManager.cpp Outdated Show resolved Hide resolved
src/cascadia/Remoting/WindowManager.h Show resolved Hide resolved
src/cascadia/WindowsTerminal/AppHost.cpp Outdated Show resolved Hide resolved
tools/tests.xml Outdated Show resolved Hide resolved
@zadjii-msft
Copy link
Member Author

@zadjii-msft zadjii-msft commented Dec 18, 2020

@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.

@github-actions

This comment has been minimized.

Copy link

@github-actions github-actions bot commented on e101efd Dec 18, 2020

New misspellings found, please review:

  • MULTIPLEUSE
  • REGCLS
To accept these changes, run the following commands
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/5757ec679b03a4240130c3c53766c91bbc5cd6a7.txt
.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt
.github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"AAAAA Bopomofo Checkboxes CParams cso CSV DOWNSCALE GENERATEPROJECTPRIFILE hhhh Horiz Inlines INPATHROOT intersectors Kode MAKEINTRESOURCEA mousemode msixbundle psin Remoting Reserialize reso RETROII rgus sbri scanbri scol SGRXY spe sph spherefunctions tcon UDK UDKs Unfocus Viginetting wz xe xlang XWV xyw yzx "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/e101efd11dba93208bba9c4191c937eabbbb4ed3.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"checkboxes csv horiz inlines MULTIPLEUSE REGCLS remoting reserialize udk unfocus "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/e101efd11dba93208bba9c4191c937eabbbb4ed3.txt is added to your repository...'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

Copy link
Member

@carlos-zamora carlos-zamora left a comment

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

_root->ProcessStartupActions(actions, false);
}

return result; // TODO:MG does a return value make sense

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

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?

_peasant = *p;
_monarch.AddPeasant(_peasant);

// TODO:MG Spawn a thread to wait on the monarch, and handle the election

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

Suggested change
// TODO:MG Spawn a thread to wait on the monarch, and handle the election
// TODO:projects/5 Spawn a thread to wait on the monarch, and handle the election

?

@@ -0,0 +1,106 @@
#include "pch.h"

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

nit: missing copyright header

void WindowManager::_createMonarch()
{
// Heads up! This only works because we're using
// "metadata-based-marshalling" for our WinRT types. THat means the OS is

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

Suggested change
// "metadata-based-marshalling" for our WinRT types. THat means the OS is
// "metadata-based-marshalling" for our WinRT types. That means the OS is
auto kingPID = _monarch.GetPID();
auto ourPID = GetCurrentProcessId();
Comment on lines +85 to +86

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

Suggested change
auto kingPID = _monarch.GetPID();
auto ourPID = GetCurrentProcessId();
const auto kingPID { _monarch.GetPID() };
const auto ourPID { GetCurrentProcessId() };
//
// I'm sure there's a better way to do this with WRL, but I'm not familiar
// enough with WRL to know for sure.
Comment on lines +11 to +13

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

nit: you could probably remove this last part haha

@@ -0,0 +1,12 @@
import "Peasant.idl";

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

nit: copyright header

@@ -0,0 +1,62 @@
#pragma once

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

nit: copyright header

@@ -0,0 +1,134 @@
#include "pch.h"

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

nit: copyright header

{
// TODO:projects/5 This is terrible. There's gotta be a better way
// of finding the first opening in a non-consecutive map of int->object
auto providedID = peasant.GetID();

This comment has been minimized.

@carlos-zamora

carlos-zamora Dec 24, 2020
Member

Suggested change
auto providedID = peasant.GetID();
const auto providedID = peasant.GetID();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.