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

Another fix for the localtests, March 2021 edition #9660

Merged
2 commits merged into from Mar 30, 2021

Conversation

zadjii-msft
Copy link
Member

Broadly, the tests were broken by #7489 because there were no _startupActions. They relied on the removed codepath that assumed wt.exe always set actions, or AppCommandlineArgs::ValidateStartupCommands created one by default.

  Broadly, the tests were broken by #7489 because there were no _startupActions. They relied on the removed codepath that assumed wt.exe always set actions, or `AppCommandlineArgs::ValidateStartupCommands` created one by default.

  These tests are still broken:
  * `TerminalAppLocalTests::TabTests::TryZoomPane [Failed]`
  * `TerminalAppLocalTests::TabTests::MoveFocusFromZoomedPane [Failed]`
  * `TerminalAppLocalTests::TabTests::CloseZoomedPane [Failed]`

  Re:#9659
@ghost ghost added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Meta The product is the management of the products. labels Mar 30, 2021
Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks bruh.

@zadjii-msft zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Mar 30, 2021
@DHowett
Copy link
Member

DHowett commented Mar 30, 2021

@msftbot merge this in like 1 minute

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Mar 30, 2021
@ghost
Copy link

ghost commented Mar 30, 2021

Hello @DHowett!

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 Tue, 30 Mar 2021 20:37:47 GMT, which is in 1 minute

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

@ghost ghost merged commit 5ab78fc into main Mar 30, 2021
@ghost ghost deleted the deb/migrie/b/9659-tests-march-21 branch March 30, 2021 20:38
ghost pushed a commit that referenced this pull request Apr 2, 2021
## Summary of the Pull Request

This PR adds support for renaming windows.

![window-renaming-000](https://user-images.githubusercontent.com/18356694/113034344-9a30be00-9157-11eb-9443-975f3c294f56.gif)
![window-renaming-001](https://user-images.githubusercontent.com/18356694/113034452-b5033280-9157-11eb-9e35-e5ac80fef0bc.gif)


It does so through two new actions:
* `renameWindow` takes a `name` parameter, and attempts to set the window's name
  to the provided name. This is useful if you always want to hit <kbd>F3</kbd>
  and rename a window to "foo" (READ: probably not that useful)
* `openWindowRenamer` is more interesting: it opens a `TeachingTip` with a
  `TextBox`. When the user hits Ok, it'll request a rename for the provided
  value. This lets the user pick a new name for the window at runtime.

In both cases, if there's already a window with that name, then the monarch will
reject the rename, and pop a `Toast` in the window informing the user that the
rename failed. Nifty!

## References
* Builds on the toasts from #9523
* #5000 - process model megathread

## PR Checklist
* [x] Closes https://github.com/microsoft/terminal/projects/5#card-50771747
* [x] I work here
* [x] Tests addded (and pass with the help of #9660)
* [ ] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

I'm sending this PR while finishing up the tests. I figured I'll have time to sneak them in before I get the necessary reviews.

> PAIN: We can't immediately focus the textbox in the TeachingTip. It's
> not technically focusable until it is opened. However, it doesn't
> provide an even tto tell us when it is opened. That's tracked in
> microsoft/microsoft-ui-xaml#1607. So for now, the user _needs_ to
> click on the text box manually.
> We're also not using a ContentDialog for this, because in Xaml
> Islands a text box in a ContentDialog won't recieve _any_ keypresses.
> Fun!

## Validation Steps Performed

I've been playing with 

```json
        { "keys": "f1", "command": "identifyWindow" },
        { "keys": "f2", "command": "identifyWindows" },
        { "keys": "f3", "command": "openWindowRenamer" },
        { "keys": "f4", "command": { "action": "renameWindow", "name": "foo" } },
        { "keys": "f5", "command": { "action": "renameWindow", "name": "bar" } },
```

and they seem to work as expected
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Second It's a PR that needs another sign-off Priority-1 A description (P1) Product-Meta The product is the management of the products.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The LocalTests are broken, March 2021 edition
3 participants