These were introduced by:
build warning with using wrong type in wil macro: #1105
namespace issues: #955
These showed up in the official Windows build. I fixed them on that side to restore the build and now I'm bringing them back out.
[Git2Git] Merged PR 3344233: Fix build warnings and namespace issues introduced by GitHub merge
Related work items: #18974333 Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_acioss 76d61f82da64f58b615a9a7f1528f0e55443777e
Related work items: #18974333
Change ParseNext function in UTF16 parser to never yield invalid data… (GH1129)
The solution here isn't perfect and isn't going to solve all of our problems. I was basically trying to stop the crash while not getting in the way of the other things coming down the pipe for the input channels.
I considered the following:
1. Remove the fail fast assertion from the buffer
- I didn't want to do this because it really is invalid to get all the way to placing the text down into the buffer and then request a string of 0 length get inserted. I feel the fail fast is a good indication that something is terribly wrong elsewhere that should be corrected.
2. Update the UTF16 parser in order to stop returning empty strings
- This is what I ultimately did. If it would ever return just a lead, it returns �. If it would ever return just a trail, it returns �. Otherwise it will return them as a pair if they're both there, or it will return a single valid codepoint. I am now assuming that if the parse function is being called in an Output Iterator and doesn't contain a string with all pieces of the data that are needed, that someone at a higher level messed up the data, it is in valid, and it should be repaired into replacements.
- This then will move the philosophy up out of the buffer layer to make folks inserting into the buffer identify half a sequence (if they're sitting on a stream where this circumstance could happen... one `wchar_t` at a time) and hold onto it until the next bit arrives. This is because there can be many different routes into the buffer from many different streams/channels. So buffering it low, right near the insertion point, is bad as it might pair loose `wchar_t` across stream entrypoints.
3. Update the iterator, on creating views, to disallow/transform empty strings.
- I considered this solution as well, but it would have required, under some circumstances, a second parsing of the string to identify lead/trail status from outside the `Utf16Parser` class to realize when to use the � character. So I avoided the double-parse.
4. Change the cooked read classes to identify that they pulled the lead `wchar_t` from a sequence then try to pull another one.
- I was going to attempt this, but @adiviness said that he tried it and it made all sorts of other weirdness happen with the edit line.
- Additionally, @adiviness has an outstanding series of effort to make cooked read significantly less horrible and disgusting. I didn't want to get in the way here.
5. Change the `GetChar` method off of the input buffer queue to return a `char32_t`, a `wstring_view`, transform a standalone lead/trail, etc.
- The `GetChar` method is used by several different accessors and API calls to retrieve information off of the input queue, transforming the Key events into straight up characters. To change this at that level would change them all. Long-term, it is probably warranted to do so as all of those consumers likely need to become aware of hand ...
Related work items: #20990158
…. It will return a replacement character at that point if it was given bad data. #788
<!-- 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 modifies the parser used while inserting text into the underlying data buffer to never return an empty sequence. The empty sequence is invalid as you can't insert a "nothing" into the buffer. The buffer asserted this with a fail fast crash. Now we will instead insert U+FFFD (the Unicode replacement character) � to symbolize that something was invalid and has been replaced.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes#788 and internal MSFT: 20990158
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [x] Requires documentation to be updated
* [x] 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: #788
<!-- 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
The solution here isn't perfect and isn't going to solve all of our problems. I was basically trying to stop the crash while not getting in the way of the other things coming down the pipe for the input channels.
I considered the following:
1. Remove the fail fast assertion from the buffer
- I didn't want to do this because it really is invalid to get all the way to placing the text down into the buffer and then request a string of 0 length get inserted. I feel the fail fast is a good indication that something is terribly wrong elsewhere that should be corrected.
2. Update the UTF16 parser in order to stop returning empty strings
- This is what I ultimately did. If it would ever return just a lead, it returns �. If it would ever return just a trail, it returns �. Otherwise it will return them as a pair if they're both there, or it will return a single valid codepoint. I am now assuming that if the parse function is being called in an Output Iterator and doesn't contain a string with all pieces of the data that are needed, that someone at a higher level messed up the data, it is in valid, and it should be repaired into replacements.
- This then will move the philosophy up out of the buffer layer to make folks inserting into the buffer identify half a sequence (if they're sitting on a stream where this circumstance could happen... one `wchar_t` at a time) and hold onto it until the next bit arrives. This is because there can be many different routes into the buffer from many different streams/channels. So buffering it low, right near the insertion point, is bad as it might pair loose `wchar_t` across stream entrypoints.
3. Update the iterator, on creating views, to disallow/transform empty strings.
- I considered this solution as well, but it would have required, under some circumstances, a second parsing of the string to identify lead/trail status from outside the `Utf16Parser` class to realize when to use the � character. So I avoided the double-parse.
4. Change the cooked read classes to identify that they pulled the lead `wchar_t` from a sequence then try to pull another one.
- I was going to attempt this, but @adiviness said that he tried it and it made all sorts of other weirdness happen with the edit line.
- Additionally, @adiviness has an outstanding series of effort to make cooked read significantly less horrible and disgusting. I didn't want to get in the way here.
5. Change the `GetChar` method off of the input buffer queue to return a `char32_t`, a `wstring_view`, transform a standalone lead/trail, etc.
- The `GetChar` method is used by several different accessors and API calls to retrieve information off of the input queue, transforming the Key events into straight up characters. To change this at that level would change them all. Long-term, it is probably warranted to do so as all of those consumers likely need to become aware of handling UTF-16 surrogates before we can declare victory. But two problems.
1. This gets in the way of @adiviness work on cooked read data
2. This goes WAY beyond the scope of what I want to accomplish here as the immediate goal is to stop the crash, not fix the world.
I've validated this by:
1. Writing some additional tests against the Utf16Parser to simulate some of the theoretical sequences that could arrive and need to be corrected into replacement characters per a verbal discussion and whiteboarding with @adiviness.
2. Manually triggered the emoji panel and inserted a bunch of emoji. Then seeked around left and right, deleted assorted points with the backspace key, pressed enter to commit, and used the up-arrow history to recommit them to see what happened. There were no crashes. The behavior is still weird and not great... but outside the scope of no crashy crashy.
Flush input queue before running test. #1137 (#1139)
Flushes the input queue on RawReadUnpacksCoalescedInputRecords test to ensure that other tests cannot cause failure by leaving extraneous input records behind after they run.
This only failed in the core operating system gate tests. This is because those tests run a subset of the complete test suite (subtracting the ones that do not make sense in a core environment). Apparently one of the tests that was skipped that normally runs prior to the UnpacksCoalesced test ensured that the input queue was clean enough for this test to succeed. But in the core environment, the test that ran prior left stuff behind.
To resolve this, I'm making the Coalesced test more resilient by cleaning out the queue prior to performing its operations.
(Also, bonus, I'm fixing the typo in the name Coalesced.)
This is less complicated/expensive than tracking down the tests that are leaving garbage behind, should prevent issues in the future related to ordering (since the tests run alphabetically, by default), and isn't as expensive as running the test in isolation (with its own conhost stood up for just the one test.)
Validated by running te.exe Microsoft.Console.Host.FeatureTests.dll /name:*InputTests* against a core operating system variant. Prior to change, this test failed. After the change, this test succeeded.
This will be automatically double-checked by the gates run after check-in.
Flushes the input queue on RawReadUnpacksCoalescedInputRecords test to ensure that other tests cannot cause failure by leaving extraneous input records behind after they run.
This only failed in the core operating system gate tests. This is because those tests run a subset of the complete test suite (subtracting the ones that do not make sense in a core environment). Apparently one of the tests that was skipped that normally runs prior to the UnpacksCoalesced test ensured that the input queue was clean enough for this test to succeed. But in the core environment, the test that ran prior left stuff behind.
To resolve this, I'm making the Coalesced test more resilient by cleaning out the queue prior to performing its operations.
(Also, bonus, I'm fixing the typo in the name Coalesced.)
This is less complicated/expensive than tracking down the tests that are leaving garbage behind, should prevent issues in the future related to ordering (since the tests run alphabetically, by default), and isn't as expensive as running the test in isolation (with its own conhost stood up for just the one test.)
Validated by running te.exe Microsoft.Console.Host.FeatureTests.dll /name:*InputTests* against a core operating system variant. Prior to change, this test failed. After the change, this test succeeded.
This will be automatically double-checked by the gates run after check-in.
Switch to using jsoncpp as our json library. This lets us pretty-print the json file by default, and lets users place comments in the json file.
We will now only re-write the file when the actual logical structure of the json object changes, not only when the serialization changes.
Unfortunately, this will remove any existing ordering of profiles, and make the order random. We don't terribly care though, because when #754 lands, this will be less painful.
It also introduces a top-level globals object to hold all the global properties, including keybindings. Existing profiles should gracefully upgrade.
Fixes#1082 -- #853's fix of the acrylic background's flash/fade on any settings change managed to cause a flash/fade on new tab creation. This change removed both flash/fades. #853 split background brush initialization from background color changes; due to the brush being constructed with a default color and then the color being initialized later, new tabs were getting the flash/fade that accompanies a re-focused fluent-style acrylic background. This PR initializes the acrylic color at brush initialization to avoid the problem.
Almost all functions in the Windows API that open or create objects and return HANDLEs to them return null on failure; only a few (mostly to do with the file system) return INVALID_HANDLE_VALUE on failure. This PR scrubs the repo of a few, but not necessarily all, cases where INVALID_HANDLE_VALUE was mistakenly used or tested against instead of null. In particular, it fixes 2 cases reported in issue #427 where the return value of CreateThread() was compared against INVALID_HANDLE_VALUE against null, causing the error handling code to run at the wrong time.
There are a lot of other uses of INVALID_HANDLE_VALUE I found that looked questionable, but which I left alone. Most of these were used to initialize HANDLE-typed variables and as a sentinel to see if those variables remained unset to a "real" value.
Fixes#427
Synchronize the font between the WDDMCon Renderer and the SCREEN_INFORMATION when the OneCore Interactivity library starts up. #21717424
Retrieved from rs_onecore_dep_acioss ccca0315e7db34c09f5fcd9dfabae666ede1687b
Fixes#958.
[Git2Git] Merged PR 3330475: Synchronize the font between the WDDMCon Renderer and the SCREEN_INFORMATION
Synchronize the font between the WDDMCon Renderer and the SCREEN_INFORMATION when the OneCore Interactivity library starts up. #21717424
Related work items: #21717424 Retrieved from https://microsoft.visualstudio.com os OS official/rs_onecore_dep_acioss ccca0315e7db34c09f5fcd9dfabae666ede1687b
Related work items: #21717424
* Specified the destructor of TerminalInput as default
* Simplified GetKeymappingLength
* Simplified GetKeyMapping
* Removed a redundant assignment
* Added auto deduction to some variables
* Merged the public sections of TerminalInput
* Implied the destructor for TerminalInput
* Removed GetKeyMappingLength and GetKeyMapping from public interface
Rearranged public section to be above private.
* Deleted or defaulted all six special member functions.
* Removed extraneous newlines
* Deleted all move and copy operations.
The default constructor is also deleted.
The destructor is defaulted.
* Converted tabs to 4 spaces
* Removed using namespace directive from header files and put these in cpp files where they are used
* Fixed tabbing issues by replacing them with spaces.
Also regrouped the using directives.
* Update src/host/exemain.cpp
Co-Authored-By: Mike Griese <migrie@microsoft.com>
* Update src/interactivity/win32/find.cpp
Co-Authored-By: Mike Griese <migrie@microsoft.com>
* Highlight the default profile in new-tab flyout
This commit makes the default profile in the new-tab flyout to show up
at the top and gives it bold text.
* Apply [[nodiscard]] to functions returning error codes
- applied [[nodiscard]] for all HRESULT, LRESULT, and NTSTATUS functions
- fixed IntelliSense declaration complaints leading to function not
implemented warnings
- deleted declared but never implemented functions
- fixed unused parameter warnings
How verified:
- bcz dbg
- opencon
- testcon
- VS2019 debug build
* - use LOG_IF_FAILED where applicable
- remove use of goto
- make MakeAltRasterFont return void
* - add missing [[nodiscard]]
- remove vestigal function declarations
- fix inconsistent function declaration
* Initial code check in for background images
* Cleaning up whitespace
* Whitespace cleanup
* Added/fixed comments
* Fixing tabs
* Reverting erroneous file add
* Removing custom enum for image stretching mode and using Windows::UI::Xaml::Media::Stretch instead.
* Removing now-superfluous static_cast when setting stretch mode.
* Updating code to use wstring_view (per #925)
* One last set of wstring -> wstring_view changes
* Split off brush-intialization function from TermControl::_BackgroundColorChanged and added code to prevent flicker on resetting acrylic or image backgrounds.
Since version 5.0.2, NuGet has used the PATH environment variable
to find MSBuild.exe before looking in other file paths.
See NuGet change
21f2b07f2c
(https://github.com/NuGet/NuGet.Client/pull/2687 ).
Unfortunately, in PR
https://github.com/microsoft/terminal/pull/606 ,
`tools\razzle.cmd` was changed to add the MSBuild.exe folder path
in _quotes_ to the PATH environment variable.
Windows itself is fine with this (you can type `msbuild` and
MSBuild runs), but some tools are not, including NuGet itself,
so you would get errors like this:
```
D:\GitHub\metathinker\console> where nuget
C:\ProgramData\chocolatey\bin\nuget.exe
D:\GitHub\metathinker\console\dep\nuget\nuget.exe
D:\GitHub\metathinker\console> nuget restore OpenConsole.sln
Illegal characters in path.
```
`razzle.cmd` runs NuGet itself, but does so before adding
the MSBuild folder to the PATH, so it was not affected by this
problem.
This change fixes the issue by dequotifying the PATH,
so that if you already had a newer version of NuGet on your PATH
before running `tools\razzle.cmd`, that version will continue
to work should you need to run `nuget restore` again
(such as after a `git clean -dx`).
Even wil::com_ptr_nothrow can still inadvertantly throw an 'access violation exception' when null pointer deref-ing (WIL won't check if it's null before attempting, CComQIPtr apparently didn't care.
Related work items: #21776133, #21781836
Even wil::com_ptr_nothrow can still inadvertantly throw an 'access violation exception' when null pointer deref-ing (WIL won't check if it's null before attempting, CComQIPtr apparently didn't care.
* cfc72cee (origin/dev/duhowett/ibxint, github/master) Make sure cursor blinks after opening new tab (1030)
* 9ad25440Fix#936: misuse of uninitialized objects causes AppVerifier breaks on Windows Terminal startup (1015)
* 5f938a04 Update Terminal.cpp (1034)
* 4c47631b Cleanup - termDispatch.hpp & adaptDispatch.hpp overrides (1004)
* cc304759 add audit mode to ci (948)
* 80f10796 Fix the bell sound when Alt+key is pressed. (1006)
* 42e87ed3 fix build break from using `await` instead of `co_await` (1009)
* 40b557a4 Update manifest to correct 1903 version, unref param fix (1008)
* 0f62ec81 Eat all tap keypresses no matter what. (985)
* ce0eaab9 inbox: Merge accumulated build fixes from RS_ONECORE_DEP_ACIOSS (1002)
* 1c509683 add .editorconfig file (585)
* efd69990 Add support for OSC 10 and 11 to set the default colors (891)
Related work items: #21610659, #21838182
* move the render thread init up; gets rid of verifier stops
* s/INVALID_HANDLE_VALUE/NULL/g since CreateEvent() and CreateThread() return a NULL HANDLE on failure; resolves another cause of AppVerifier breaks
* Fix for https://github.com/microsoft/terminal/issues/896
* Fixing spaces
* Base Class destructor is virtual, derived class destructor shouldn't be declared vitual or override
* Update src/terminal/adapter/termDispatch.hpp
nit: remove space
Co-Authored-By: Mike Griese <migrie@microsoft.com>
* Update manifest to correct 1903 version
While messing around with building with VS2019/14.2/etc, I noticed that the version we're using in the compatibility manifest doesn't match guidance for XAML Islands. This puts the version information from the public guidance into the manifest and leaves a link back to the page where I got this idea from.
* comment out unused params in IslandWindow::OnResize
* Merged PR 3302855
[Git2Git] Git Train: Merge of building/rs_onecore_dep_acioss/190523-1700 into official/rs_onecore_dep_acioss Retrieved from official/rs_onecore_dep_acioss 3fceea90bee761aa93d91c0184a7217d1e2d404b
Related work items: #18974333
[Git2Git] Git Train: Merge of building/rs_onecore_dep_acioss/190523-1700 into official/rs_onecore_dep_acioss Retrieved from official/rs_onecore_dep_acioss 3fceea90bee761aa93d91c0184a7217d1e2d404b
Related work items: #18974333
* Support OSC to set default background and foreground colors
* Update the Terminal theme when the background changes
* Fix whitespace per code-review
* Add Documentation Comments
Also fix a few outdated comments and whitespace
* Update Telemetry codes per code review
* Add Unit Tests for OSC ForegroundColor and BackgroundColor
* Add a couple additional test cases
* Minor doc and whitespace change per PR review
* Update comment help per code review
* Add another OSC 10 & 11 test case, improve output
* Comments and syntax cleanup per code reviews
Fix a bunch of static analysis issues (GH553)
* static analysis fixes
* using C++ style casts
* explicit delete changed to reset(nullptr)
* fix for null apiMsg.OtherId during tracing in Compare()
* changed INVALID_ID macro to constexpr
* properly handle null ReplyMsg in ConsoleIoThread()
* Fixed wrong static_cast for State.InputBuffer
* compensate for null reply message to fix deref problem of ReplyMsg in srvinit.cpp by changing signature in DeviceComm.h
Related work items: #21767097
* added another method to scroll with keyboard
* set lastscrolloffset to 0
* fixed unused variable
* renamed ViewPort to Viewport
* changed keyBoard to keyboard in the functions, and added expliantion for function
