8145 - diff between phase 0 and phase 1 #5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This PR adds shortcut action so that users can scroll. I used `UINT16_MAX` for `rowsToScroll`. <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes microsoft#7542 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed
A bunch of our feature tests don't work reliably in CI. They rely on creating a new `OpenConsole.exe` window, then running the test _in that console_. As a part of that test setup, the test runner used to wait a second to attach to the newly created console. Then the test goes on it's merry way, assuming the console is ready to go. However, in CI, that might take more than a second. If it does, then the test would fail pretty immediately, as soon as it tries to get at the buffer of the new console. This PR introduces a little retry loop to the test init. After attaching to the new console, we'll try and get at the screen buffer. If that fails, we'll wait a second and try again. We'll try 5 times total, before bailing entirely. Hopefully, this should mitigate most of the random CI failures we get in the feature tests. Closes microsoft#8495
We've moved to the recommended internal code signing service and no longer need these configurations.
So the implementation is somewhat dirty. The ideas was nice - add lostFocusHandler However it broke few things: * In the TabSwitcher the ListItem must be focusable since otherwise the SingleSelectionMode behavior breaks. To address this I had to put the lostFocusHandler on the items as well * When you click the flyout, the palette loses focus, which makes the terminal page to set the focus on the tab, closing the flyout. To address this I had to ensure the tab won't get focused once the flyout is open. In addition, flyout should fix the focus before opening, otherwise alt+tab will put a focus on a tab row rather than on tab * I also had to close the palette if the tab order changes. To prevent inconsistencies. Closes microsoft#8355
…microsoft#8420) First step towards microsoft#8415: * Introduce `PaletteItem` and derive from it to provide native support for tabs and command lines (`ActionPaletteItem` / `TabPaletteItem`, `CommandLinePaltteItem`) * Remove business logic behind PaletteItem from palette (aka dispatch commands and preview tabs externally)
…rosoft#8521) If a single line of text is selected, use it to populate the search box. Closes microsoft#8307
Closes microsoft#8419 Co-authored-by: KiminoriKaburagi <[email protected]>
We've got similar logging around the command palette, we really should have the same logging on the tab renamer as well. ## PR Checklist * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed  Look, it works
…t#8547) * The index field should be of type `"null"` and not `null`. * The default value should be `null` and not `""` Closes microsoft#8024
I added `enum class` to one thing and decided that that was quite enough before disabling the `enum class` warning. Looks like 16.8 made more map/vector operations noexcept, so we have to re-annotate to remain compliant.
Adds a "move to previous pane" and "move to next pane" keybinding, which navigates to the last/first focused pane We assign pane IDs on creation and maintain a vector of active pane IDs in MRU order. Navigating panes by MRU then requires specifying which pane ID we want to focus. From our offline discussion (thanks @zadjii-msft for the concise description): > For the record, the full spec I'm imagining is: > > { command": { "action": "focus(Next|Prev)Pane", "order": "inOrder"|"mru", "useSwitcher": true|false } }, > > and order defaults to mru, and useSwitcher will default to true, when > there is a switcher. So > > { command": { "action": "focusNextPane" } }, > { command": { "action": "focusNextPane", "order": "mru" } }, > > these are the same action. (but right now we don't support the order > param) > > Then there'll be another PR for "focusPane(target=id)" > > Then a third PR for "focus(Next|Prev)Pane(order=inOrder)" > for the record, I prefer this approach over the "one action to rule > them all" version with both target and order/direction as params, > because I don't like the confusion of what happens if there's both > target and order/direction provided. References microsoft#1000 Closes microsoft#2871
Spec on how we display profile inheritance inside the settings UI. [Markdown View](https://github.com/microsoft/terminal/blob/dev/cazamor/spec/sui-inheritance/doc/specs/%231564%20-%20Settings%20UI/cascading-settings.md) ## References microsoft#1564 - Settings UI
## PR Checklist * [x] Closes microsoft#7916 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [ ] I've discussed this with core contributors already. ## Detailed Description of the Pull Request / Additional comments Upon tab close the tabview is responsible to issue tab selection for the next active tab. However this doesn't happen when tabview is hidden. There was a special treatment for this scenario for full screen mode. Added the same treatment to focus mode (as the tabview is not visible in this case as well). ## Validation Steps Performed Manual tests
This PR defines a series of `NOSOMETHING` macros in PCHs, in order to prevent `windows.h` from bringing a lot of rarely used things into the project. Theoretically this should make PCH generation and overall complication faster, but I didn't really benchmark the speed. Another benefit would be reducing the symbol noises caused by `windows.h`.
The Settings UI will need to take a dependency on IconSourceConverter.
The settings UI will eventually need to know about the available enum values for a given field. This offers that capability.
This commit is an amalgamation of some of the TSM changes in PR microsoft#8048. It: * Introduces CascadiaSettings.CreateNewProfile to add a new profile * Introduces CascadiaSettings.ProfileDefaults, which returns the "defaults" object as a profile * Fixes the font weight deserializer to work on uint16_ts * Fixes a property getter in ColorScheme to not be a property getter * Fixes a reserialization error with default profiles * Sets a default icon for all profiles (to the C:\ Segoe MDL2 icon)
This commit introduces the terminal settings editor (to wit: the Settings UI) as a standalone project. This project, and this commit, is the result of two and a half months of work. TSE started as a hackathon project in the Microsoft 2020 Hackathon, and from there it's grown to be a bona-fide graphical settings editor. There is a lot of xaml data binding in here, a number of views and a number of view models, and a bunch of paradigms that we've been reviewing and testing out and designing and refining. Specified in microsoft#6720, microsoft#8269 Follow-up work in microsoft#6800 Closes microsoft#1564 Closes microsoft#8048 (PR) Co-authored-by: Carlos Zamora <[email protected]> Co-authored-by: Kayla Cinnamon <[email protected]> Co-authored-by: Alberto Medina Gutierrez <[email protected]> Co-authored-by: John Grandle <[email protected]> Co-authored-by: xerootg <[email protected]> Co-authored-by: Scott <[email protected]> Co-authored-by: Vineeth Thomas Alex <[email protected]> Co-authored-by: Leon Liang <[email protected]> Co-authored-by: Dustin L. Howett <[email protected]> Signed-off-by: Dustin L. Howett <[email protected]>
…t#8048) This commit iontroduces another `target` to the `openSettings` binding: `settingsUI`. It opens the settings UI introduced in the previous commit. Closes microsoft#1564 Closes microsoft#8048 (PR) Co-authored-by: Carlos Zamora <[email protected]> Co-authored-by: Leon Liang <[email protected]>
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
It is maybe not the best way since I had to get all the cases for key handling so I just created for each of them. As a result the code get longer(not optimized). Most difficult thing was Next tab and Previous tab I just could not solve it.
### 9 commands that couldn't enabled > <
1. Increase font size -> could not find VirtualKey for "-"
2. Decrease font size -> could not find VirtualKey for "+"
3. Split pane, split:horizontal -> could not find VirtualKey for "-"
4. Split pane, split:vertical -> could not find VirtualKey for "+"
5. Open default settings -> could not find VirtualKey for ","
6. Open settings file -> could not find VirtualKey for ","
7. Open new tab dropdown -> could not resolve keybindings
8. Next tab -> could not resolve keybindings
9. Previous tab -> could not resolve keybindings
<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? -->
## References
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes microsoft#6679
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [ ] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx
<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
This changes the keyboard warning from a dialog to an `InfoBar`, which we just got in MUX 2.5. Some users were unhappy that we'd always display the dialog. We learned from the input team that this service _should_ always be enabled. We're also learing from users that they don't always want it enabled. We're working with the Input team to help us figure out how this service can be disabled _and the Terminal work just fine_. They're confident that it _shouldn't_. For 99% of our users, they're right. So we don't want to get rid of the dialog entirely, we want to understand how this is possible. While we wait, let's make the message less aggressive. This is instead of making a `iKnowWhatImDoingDisableTheKeyboardWarning` setting to disable the dialog. Props to @cornem for suggesting the less aggressive solution. ## Validation Steps Performed Tested manually, but by forcing the message to always display. Disabling the service requires two full reboots, and _ain't nobody got time for that_. Closes microsoft#8228 Closes microsoft#4448, for now
This reverts commit 1965fb5.
In conhost there is a keyboard shortcut that applies colors to the selected range of text, and another shortcut that searches for the selected text, and applies colors to any matching content. The former operation doesn't work correctly when the selection is wrapped, and both have problems when the selected text spans DBCS characters. This PR attempts to fix those issues. The problem with the color section was that it applied the color to a simple rect spanning the start and end points of the selection. I've now updated it to use the `Selection::GetSelectionRects` method, which correctly handles a wrapped range of lines, and makes sure that double width characters are fully covered. The problem with the "find-and-color" operation was in the way it obtained the search text from the selected screen cells. Since it retrieved one cell at a time, and a DBCS character can span two cells, that resulted in some characters being repeated in the search text. I've now corrected that code to take the width of the characters into account. ## Validation Steps Performed I've manually verified that the test cases described in microsoft#8572 and microsoft#8574 are now working correctly. Closes microsoft#8572 Closes microsoft#8574
…microsoft#8579) There are two issue with copy to clipboard when block is selected: * We don't add new lines for lines that were wrapped * We remove trailing whitespaces which is not intuitive in block selection. Fixed the copy logic to always add newlines and not to remove whitespaces when block is selected. Even if shift is pressed! ## Detailed Description of the Pull Request / Additional comments * Added optional parameter to `TextBuffer::GetText` that allows to apply formatting (includeCRLF / trimming) to lines that were wrapped * Changed `Terminal::RetrieveSelectedTextFromBuffer` to apply the following parameters when block is selected: * includeCRLF = true * trimTrailingWhitespaces = false * apply the formatting above to all rows, including the ones that were wrapped ## Validation Steps Performed * Manual tests for both block and standard selection * Copy with both right-click and command * Added UT Closes microsoft#6740
Co-authored-by: mrange <[email protected]> I loved the pixel shaders in microsoft#7058, but that PR needed a bit of polish to be ready for ingestion. This PR is almost _exactly_ that PR, with some small changes. * It adds a new pre-profile setting `"experimental.pixelShaderPath"`, which lets the user set a pixel shader to use with the Terminal. - CHANGED FROM microsoft#7058: It does _not_ add any built-in shaders. - CHANGED FROM microsoft#7058: it will _override_ `experimental.retroTerminalEffect` * It adds a bunch of sample shaders in `samples/shaders`. Included: - A NOP shader as a base to build from. - An "invert" shader that inverts the colors, as a simple example - An "grayscale" shader that converts all colors to grayscale, as a simple example - An "raster bars" shader that draws some colored bars on the screen with a drop shadow, as a more involved example - The original retro terminal effects, as a more involved example - It also includes a broken shader, as an example of what heppens when the shader fails to compile - CHANGED FROM microsoft#7058: It does _not_ add the "retroII" shader we were all worried about. * When a shader fails to be found or fails to compile, we'll display an error dialog to the user with a relevant error message. - CHANGED FROM microsoft#7058: Originally, microsoft#7058 would display "error bars" on the screen. I've removed that, and had the Terminal disable the shader entirely then. * Renames the `toggleRetroEffect` action to `toggleShaderEffect`. (`toggleRetroEffect` is now an alias to `toggleShaderEffect`). This action will turn the shader OR the retro effects on/off. `toggleShaderEffect` works the way you'd expect it to, but the mental math on _how_ is a little weird. The logic is basically: ``` useShader = shaderEffectsEnabled ? (pixelShaderProvided ? pixelShader : (retroEffectEnabled ? retroEffect : null ) ) : null ``` and `toggleShaderEffect` toggles `shaderEffectsEnabled`. * If you've got both a shader and retro enabled, `toggleShaderEffect` will toggle between the shader on/off. * If you've got a shader and retro disabled, `toggleShaderEffect` will toggle between the shader on/off. References microsoft#6191 References microsoft#7058 Closes microsoft#7013 Closes microsoft#3930 "Add setting to retro terminal shader to control blur radius, color" Closes microsoft#3929 "Add setting to retro terminal shader to enable drawing scanlines" - At this point, just roll your own version of the shader.
…ft#8367) This commit replaces DeviceComm with the interface IDeviceComm and the concrete implementation type ConDrvDeviceComm. This work is done in preparation for different device backends. In addition to separating out ConDrv-specific behavior, I've introduced a "handle exchange" interface. HANDLE EXCHANGE --------------- There are points where we give ConDrv opaque handle identifiers to our input buffer, output buffer and process data. The exact content of the opaque identifier is meaningless to ConDrv: the driver's only interaction with these identifiers is to hold onto them and send back whichever are pertinent for each API call. Because of that, we used the raw register-width pointer value of the input buffer, output buffer or process data _as_ the opaque handle value. This works very well for ConDrv <-> conhost using the ConDrvDeviceComm. It works less well for something like the "logging" DeviceComm that will log packets to a file: those packets *cannot* contain pointer values (!) To address this, and to afford flexibility to DeviceComm implementers, I've introduced a two-member complement of handle management functions: * `ULONG_PTR PutHandle(void*)` registers an object with the DeviceComm and returns an opaque identifier. * `void* GetHandle(ULONG_PTR)` takes an opaque identifier and returns the original object. ConDrvDeviceComm implements PutHandle and GetHandle by casting the object pointer to the "opaque handle value", which maintains wire format compatibility[1] with revisions of conhost prior to this change. Simpler DeviceComm implementations that require handle tracking but cannot bear raw pointers can implement these functions by returning an autoincrementing ID (or similar) and registering the raw object pointer in a mapping. I've audited all existing handle exchanges with the driver and updated them to use Put/GetHandle. (I intended to add DestroyHandle, but we are not equipped for handle removal at the moment. ConsoleHandleData/ConsoleProcessHandle are destroyed during wait routine completion, on client disconnect, etc. This does mean that an id<->pointer mapping will grow without bound, but at a cost of ~8 bytes per entry and a short-lived console session I am not too concerned about the cost.) [1] Wire format compatibility is not required, and later we may want to switch ConDrvDeviceComm to `EncodePointer` and `DecodePointer` just to insulate us against a spurious ConDrv packet allowing for an arbitrary 4/8-byte read and subsequent liftoff into space.
This commit introduces another optional text block in palette that will be shown in the command line mode (above the history). This text block will either contain a list of parsed command lines or a description why the parsing failed Closes microsoft#8344 Closes microsoft#7284
This commit adds a [progress ring] to the tab header when we receive an OSC 9 sequence. Adds an event handler in `Tab.cpp` for the event we raise when we get a request to set the taskbar state/progress. This event handler updates the tab header with the active control's state/progress. When we want to show the progress ring, we hide the tab icon and place the progress ring over it. [progress ring]: https://docs.microsoft.com/en-us/uwp/api/Microsoft.UI.Xaml.Controls.ProgressRing?view=winui-2.4 References microsoft#6700
…icrosoft#8489) I was looking at conhost/OpenConsole and noticed it was being pretty inefficient with allocations due to some usages of std::deque and std::vector that didn't need to be done quite that way. So this uses std::vector for the TextBuffer's storage of ROW objects, which allows one allocation to contiguously reserve space for all the ROWs - on Desktop this is 9001 ROW objects which means it saves 9000 allocations that the std::deque would have done. Plus it has the benefit of increasing locality of the ROW objects since deque is going to chase pointers more often with its data structure. Then, within each ROW there are CharRow and ATTR_ROW objects that use std::vector today. This changes them to use Boost's small_vector, which is a variation of vector that allows for the so-called "small string optimization." Since we know the typical size of these vectors, we can pre-reserve the right number of elements directly in the CharRow/ATTR_ROW instances, avoiding any heap allocations at all for constructing these objects. There are a ton of variations on this "small_vector" concept out there in the world - this one in Boost, LLVM has one called SmallVector, Electronic Arts' STL has a small_vector, Facebook's folly library has one...there are a silly number of these out there. But Boost seems like it's by far the easiest to consume in terms of integration into this repo, the CI/CD pipeline, licensing, and stuff like that, so I went with the boost version. In terms of numbers, I measured the startup path of OpenConsole.exe on my dev box for Release x64 configuration. My box is an i7-6700k @ 4 Ghz, with 32 GB RAM, not that I think machine config matters much here: | | Allocation count | Allocated bytes | CPU usage (ms) | | ------ | ------------------- | ------------------ | -------------- | | Before | 29,461 | 4,984,640 | 103 | | After | 2,459 (-91%) | 4,853,931 (-2.6%) | 96 (-7%) | Along the way, I also fixed a dynamic initializer I happened to spot in the registry code, and updated some docs. ## Validation Steps Performed - Ran "runut", "runft" and "runuia" locally and confirmed results are the same as the main branch - Profiled the before/after numbers in the Visual Studio profiler, for the numbers shown in the table Co-authored-by: Austin Lamb <[email protected]>
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request The tab tooltip is no longer empty when it was toggle zoomed. <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes microsoft#8199 * [ ] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.