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

Teach terminal to hide mouse while typing #8629

Merged
8 commits merged into from
Jan 21, 2021

Conversation

Don-Vito
Copy link
Contributor

@Don-Vito Don-Vito commented Dec 21, 2020

PR Checklist

Detailed Description of the Pull Request / Additional comments

  • Use SPI_GETMOUSEVANISH setting
  • Hide upon CharacterReceived
  • Unhide upon PointerMoved, PointerMoved, MouseWheel, LoseFocus

@ghost ghost added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Dec 21, 2020
@Don-Vito Don-Vito marked this pull request as draft December 21, 2020 00:31
@github-actions
Copy link

New misspellings found, please review:

  • GETMOUSEVANISH
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/e1d615aa30d357fac68ac3bcafc560ca222994da.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"checkboxes csv GETMOUSEVANISH horiz inlines 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/e1d615aa30d357fac68ac3bcafc560ca222994da.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.

@Don-Vito Don-Vito changed the title Teach terminal control to hide mouse while typing Teach terminal to hide mouse while typing Dec 21, 2020
@Don-Vito Don-Vito marked this pull request as ready for review December 21, 2020 02:09
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Implementation is pretty solid. Thanks!

I'm wondering if we should make this a setting? Or maybe we should test this out in WT Preview a bit to see to what level people want to configure it? Either way, I'm ok merging it as-is, then deciding on it's configurability later.

src/cascadia/TerminalControl/TermControl.idl Outdated Show resolved Hide resolved
@Don-Vito
Copy link
Contributor Author

Don-Vito commented Dec 24, 2020

Implementation is pretty solid. Thanks!

I'm wondering if we should make this a setting? Or maybe we should test this out in WT Preview a bit to see to what level people want to configure it? Either way, I'm ok merging it as-is, then deciding on it's configurability later.

I didn't introduce a setting because we already read a system wide one. So I suggest trying this in preview and seeing what happens 😊

@zadjii-msft zadjii-msft assigned zadjii-msft and DHowett and unassigned zadjii-msft Jan 4, 2021
@zadjii-msft
Copy link
Member

You know what, @DHowett has spent a lot of time working on our mouse pointer handling before, so I'm gonna make sure he's the second signoff for this.

@msftbot make sure @DHowett signs off on this

@DHowett
Copy link
Member

DHowett commented Jan 4, 2021

Systemwide setting >>> local one, always.

EDIT: i mean, we do not need a local setting for this

@Don-Vito Don-Vito requested a review from DHowett January 13, 2021 11:48
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

What happens when the cursor is hidden, and then terminal exits (or the pane closes or what-have-you?) Will the next terminal focused properly restore it to visible?

@Don-Vito
Copy link
Contributor Author

What happens when the cursor is hidden, and then terminal exits (or the pane closes or what-have-you?) Will the next terminal focused properly restore it to visible?

If the terminal exits / pane is hidden, the next terminal will not show the cursor until the mouse is moved. This is somewhat inconsistent, as switching between terminals does restore the cursor immediately. I will fix it.

@DHowett
Copy link
Member

DHowett commented Jan 20, 2021

It may be fine if you don't fix it. As long as Windows restores it eventually. 😄

@Don-Vito
Copy link
Contributor Author

Don-Vito commented Jan 20, 2021

It may be fine if you don't fix it. As long as Windows restores it eventually. 😄

Fixed :) Actually it was important: open a terminal tab and settings tab, start typing in the terminal tab, then ctrl+w. Learn how to edit settings without a mouse 😄

@@ -2582,6 +2582,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
{
if (!_closing.exchange(true))
{
_RestorePointerCursorHandlers(*this, nullptr);
Copy link
Member

Choose a reason for hiding this comment

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

hmm.. will this restore the cursor to pointer if a terminal closes while a different one is the one that hid it? The sleep 5; exit scenario, but you go type in a different terminal tab while it's waiting to beef it

Copy link
Contributor Author

@Don-Vito Don-Vito Jan 20, 2021

Choose a reason for hiding this comment

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

Once again this user who wants to destroy terminal? 😄
The restore is managed on the TerminalPage level - so we won't need to remember who hid it.
If I don't miss anything, in the worst case we will get mouse restored.. while typing.. and then it will be hidden again.

Copy link
Member

Choose a reason for hiding this comment

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

Let's leave it like this. This will be okay for now  😄

Copy link
Member

Choose a reason for hiding this comment

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

user who wants to destroy terminal

You'd be surprised! Look for posts by vefatica here 😉 and there's also one Microsoft person who keeps reporting bugs in multibyte character handling because he apparently just runs a stress testing program that emits random code units and then he actively resizes the window while it's doing that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You'd be surprised! Look for posts by vefatica here 😉 and there's also one Microsoft person who keeps reporting bugs in multibyte character handling because he apparently just runs a stress testing program that emits random code units and then he actively resizes the window while it's doing that

After few months following the tracker I am not surprised with anything as the creativity is limitless 😊 And I am a big fan of vefatica quite for a while - even before he found the handle leak.

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 21, 2021
@DHowett
Copy link
Member

DHowett commented Jan 21, 2021

Thanks again!

@ghost
Copy link

ghost commented Jan 21, 2021

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 9c4950c into microsoft:main Jan 21, 2021
@ghost
Copy link

ghost commented Jan 28, 2021

🎉Windows Terminal Preview v1.6.10272.0 has been released which incorporates this pull request.:tada:

Handy links:

mpela81 pushed a commit to mpela81/terminal that referenced this pull request Jan 28, 2021
## PR Checklist
* [x] Closes microsoft#5699
* [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
* Use SPI_GETMOUSEVANISH setting
* Hide upon CharacterReceived
* Unhide upon PointerMoved, PointerMoved, MouseWheel, LoseFocus
@viral32111
Copy link

Is there a settings/configuration option to disable this beaviour? I preferred it when the mouse pointer stayed visible all the time because it seems to rapidly flash between hidden & showing when typing, which gets very annoying.

Also when the cursor is hidden and you move it up to the minimise/maximise/exit buttons it doesn't reappear.

@zadjii-msft
Copy link
Member

zadjii-msft commented Mar 3, 2021

@viral32111 So, we're already checking the state of the system-wide Mouse vanish setting. So the terminal should be behaving in line with other apps. You could disable that setting if you wanted to have the cursor always visible.

Now that I'm typing this up though, it seems like Edgium doesn't vanish the cursor as I type though. So maybe there should be an additional setting.

notes:

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-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants