Scroll from selection dragging out of window (#1247) #1523
Conversation
| const short lastVisibleCol = std::max(_terminal->GetViewport().Width() - 1, 0); | ||
|
|
||
| terminalPosition.Y = std::clamp(terminalPosition.Y, short{ 0 }, lastVisibleRow); | ||
| terminalPosition.X = std::clamp(terminalPosition.X, short{ 0 }, lastVisibleCol); |
There was a problem hiding this comment.
Check out Viewport::Clamp(). Is there a way we could use that here? #Resolved
There was a problem hiding this comment.
Viewport::Clamp() expects absolute position, whereas src and dst of terminalPosition are relative to scroll offset. #Resolved
| { | ||
| if (_autoScrollVelocity != 0) | ||
| { | ||
| _scrollBar.Value(_scrollBar.Value() + _autoScrollVelocity * _AutoScrollTimerInterval); |
There was a problem hiding this comment.
I just want to say. I understand this because of Physics class and I never thought that moment would come. #Resolved
There was a problem hiding this comment.
Just everyday snippet at gamedev :) #Resolved
|
|
||
| if (_terminal->IsSelectionActive() && pointerPoint.Properties().IsLeftButtonPressed()) | ||
| { | ||
| // If user is mouse selecting and scrolls, he then points at new character. |
There was a problem hiding this comment.
| // If user is mouse selecting and scrolls, he then points at new character. | |
| // If user is mouse selecting and scrolls, they then point at new character. | |
| ``` #Resolved |
There was a problem hiding this comment.
Is this how do you write that? Well, understand the sad purpose of this, but it doesn't seem grammatically correct. #Resolved
There was a problem hiding this comment.
I agree it sounds weird; guess I just got used to it. It's one of the ways I learned it and it's generally pretty easy to incorporate. #Resolved
ghost
added
Needs-Author-Feedback
|
I've changed |
|
Looks like you need to run Invoke-CodeFormat on your branch. Re-running the CI because it said "tests failed" but this shouldn't be affecting the tests that failed. |
This comment has been minimized.
This comment has been minimized.
|
Ok that helped |
|
Actually, the middle-mouse-button (panning) scroll could be implemented using the same mechanism, unless there is a better way to do this (like the wierd function that conhost uses). Is there? |
Huh. I didn't even realize that conhost had that feature until just now. I think if we add it in though, we'd want some kind of visual cue (as in ConHost now) over the cursor. Feel free to create a Feature Request on this. |
ghost
added
the
Needs-Author-Feedback
This commit changes how TerminalControl/TerminalCore handle key events to give it better knowledge about modifier states at the lower levels.
ghost
removed
the
Needs-Author-Feedback
|
I cant stand this merging.. |
* Connects clipboard functionality to their keybindings. * Cleaning up comments and whitespace. * Added "copyTextWithoutNewlines" keybinding. * Fixing tabs in idl file * Fixing merge conflicts * Adding default keybindings for copy and paste to ctrl-shift-c and ctrl-shift-v, respectively. * Complying with refactoring * Fixing formatting issues
…ith a single Island (microsoft#929) * Use a region to cut off the dragable region * Use proper measurements for the draggable area * Working better, paint works most of the time * Fix a bug where paint is incomplete when double clicking the dragbar * Remove old fork on XamlApplication * Upgrade to XamlApp preview6.2 * Add Microsoft.VCRTForwarders to make it easy to dogfood Co-Authored-By: Michael Niksa <miniksa@microsoft.com> Co-Authored-By: Mike Griese <migrie@microsoft.com>
…osoft#1012) * move version to vs2019, the 1903 sdk, and the 14.2 build tools.
…t#1676) * App now initializes the connection instead of term control.
* Refactors TerminalApp into two projects: - TerminalAppLib, which builds a .lib, and includes all the code - TerminalApp, which builds a dll by linking the lib * Adds a TerminalApp.Unit.Tests project - Includes the ability to test cppwinrt types we've authored using a SxS manifest for unpackaged winrt activation - includes the ability to test types with XAML content using an appxmanifest * Adds a giant doc explaining how this was all done. Really, just go read that doc, it'll really help you understand what's going on in this PR. ------------------------- These are some previous commit messages. They may be helpful to future readers. * Start adding unittests for json parsing, end up creating a TerminalAppLib project to make a lib. See microsoft#1042 * VS automatically did this for me * This is a dead end I tried including the idl-y things into the lib, but that way leads insanity If you want to make a StaticLibrary, then suddenly the winrt toolchain forgets that ProjectReferences can have winmd's in them, so it won't be able to compile any types from the referenced projects. If you instead try to manually reference the types, you'll get duplicate types up the wazoo, which of course is insane, since we're referencing them the _one_ time * Yea just follow microsoft#1042 on github for status So current state: 1. If you try to add a `Reference` to all of MUX.Markup, TerminalControl and TerminalSettings, then mdmerge will complain about all the types from TerminalSettings being defined twice. In this magic scenario, the dependencies of TerminalControl are used directly for some reason: ``` 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalSettings\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.Settings.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalConnection.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\TerminalControl\Microsoft.Terminal.TerminalControl.winmd. 12> Load input metadata file ...OpenConsole\x64\Debug\Microsoft.UI.Xaml.Markup\Microsoft.UI.Xaml.Markup.winmd. ``` 2. If you don't add a `Reference` TerminalControl, then it'll complain about being unable to find the type TitleChangedEventArgs, which is defined in TerminalControl. 3. If you don't add a `Reference` TerminalSettings, then it'll complain about being unable to find the type KeyChord and other types from TerminalSettings. In this scenario, it doesn't recurse on the other dependencies from TerminalControl for whatever reason. 4. If you instead try to add all 3 as a `ProjectReference`, then it'll complain about being unable to find TitleChangedEventArgs, as in 2. Presumably, it;ll have troubles with the other types too, as none of the 3 are actually included in the midlrt.rsp file. 5. If you add all 3 as a `ProjectReference`, then also add TerminalControl as a `Reference`, you'll get a `MIDL2011: [msg] unresolved type declaration Microsoft.UI.Xaml.Markup.XamlApplication` 6. If you add all 3 as a `ProjectReference`, then also add TerminalControl AND MUX.Markup as a `Reference`, you'll get the same result as 3. * what if we just don't idl This seems to compile * This compiles but I broke the MUX resources look at the App.xaml change. in this changelist. That's what's broken right now. Lets fix that! * lets do this If I leave the MUX nuget out of the project, I'll get a compile error in App.xaml: ``` ...OpenConsole\src\cascadia\TerminalApp\App.xaml(21,40): XamlCompiler error WMC0001: Unknown type 'XamlControlsResources' in XML namespace 'using:Microsoft.UI.Xaml.Controls' ``` If I add it back to the project, it works * Some cleanup from the previous commit * This is busted again. Doing a clean build didn't work. A clean rebuild of the project, paired with some removal of dead code revealed a problem with what I have so far. TerminalAppLib depends on the generation of two headers, `AppKeyBindings.g.h` and `App.g.h`, as those define some of bits of the winrt types. They're needed to be able to compile the implementations. Presumably that's not getting generated by the lib project, because the dll project is the one to generate that file. So we need to move the idl's to the lib project. This created maddness, because of course the Duplicate Type thing. The solution to that is to actually mark the winrt DLLs that we're chaining up through us as ``` <Private>false</Private> <CopyLocalSatelliteAssemblies>false</CopyLocalSatelliteAssemblies> ``` This will prevent them from getting double-included. This still doesn't work however, since ``` app.cpp(40): error C2039: 'XamlMetaDataProvider': is not a member of 'winrt::TerminalApp' error C3861: 'XamlMetaDataProvider': identifier not found ``` So we need to figure that out. The dll project is still generating the right header, so lets look there. * Move the xaml stuff to the lib This compiles, but when we launch, we fail to load the tabviewcontrol resources again. So that's not what you want. Why is it not included? * It works again! * Use the pri, xbf files from TerminalAppLib, not TerminalApp * Manually make TerminalApp include a reference to TerminalAppLib's TerminalApp.winmd. This will force the build to copy TerminalApp.winmd to TerminalApp/, which WindowsTerminal needs to be able to ProjectReference the TerminalApp project (it's expecting it to have a winmd) * Remove the module.g.cpp from TerminalApp, and move to TerminalAppLib. The dll doesn't do any codegen anymore. * Agressively clean up these files * Clean up unnecessary includes in the dll pch.h * This does NOT work. The WindowsxamlManager call crashes. I'm thinking it has to do with activation of winrt types from a dll. Email out to @Austin-Lamb to see if he can assist * This gets our cppwinrt types working, but xaml islands is still broken * Split the tests apart, so they aren't insane * These are the magic words to make xaml islands work * All this witchcraft is necessary to make XAML+MUX work right * Clean this up a bit and add comments * Create an enormous doc explaining this madness * Unsure how this got changed. * Trying to get the CI build to work again. This resolves the MUX issue. We need to manually include it, because their package's target doesn't mark it as CopyLocalSatelliteAssemblies=false, Private=false. However, the TerminalApp project is still able to magically reason that the TerminalAppLib project should be included in the MdMerge step, because it think's it's a `GetCppWinRTStaticProjectReferences` reference. * Update cppwinrt to the latest version - this fixes the MSBuild * I still need to re-add the KeyModifiers checks from TermControl. I think this update broke `operator&` for that enum. * There needs to be some cleanup obviously * The doc should be updated as well * Clean up changes from cppwinrt update * Try doing this, even though it seems wrong * Lets try this (press x to doubt) * Clean up vcxproj file, and remove appxmanifest change from previous commit * Update to the latest TAEF release, maybe that'll work * Let's try a prerelease version, shall we? * Add notes about TAEF package, comment out tests * Format the code * Hopefully fix the arm64 and x86 builds also a typo * Fix PR nits * Fix some bad merge conflicts * Some cleanup from the merge * Well I was close to getting the merge right * I believe this will fix CI * Apply suggestions from code review Co-Authored-By: Carlos Zamora <carlos.zamora@microsoft.com> * These definitely need to be fixed * Try version detecting in the test IDK if this will build, I'm letting the CI try while I clean rebuild locally * Try blindly updating to the newest nuget version * Revert "Try blindly updating to the newest nuget version" This reverts commit b72bd9e. * We're just going to see if these work in CI with this change * Comment the tests back out. Windows Server 2019 is 10.0.17763.557 * Remove the nuget package We don't need this package anymore now that we're hosting it * Okay this _was_ important
Rebase? |
|
Rebase by the means of merging master onto branch. Or whatever. I must now do this by hand since Visual Studio's Git plugin turned out to be untrustworthy. After resolving the same conflicts few times, resolving totally unrelated conflicts, crash, and when everything looked ok, then this happend. Fortunetly there isn't much going on in this PR. Sorry for problems. |
Sorry. I always do this with git commands at the command line, not with the Visual Studio plugin. I've been burned by it before too. Apologies. :( |
|
Good word github did a number on this |
|
Yea, now it's one of the biggest PRs ever. |
|
Finally! |
|
OK @mcpiroman, I'll save you from more misery here and get it merged. Thank you for bearing with all of that. |
|
🎉 Handy links: |
ghost
mentioned this pull request
Summary of the Pull Request
When user is mouse selecting and cursor wents out of viewport, terminal now scrolls down or up to 'follow' the cursor and expands selection. Additionally, when scrolling with mouse wheel while selecting, selection now updates immediately (instead of on next mouse move event).
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
DispatcherTimer. Idk if it's prefered solution but I'm not aware of better one.Validation steps