Fix text selection while new lines are being printed when history buffer is full #10749

PankajBhojwani · 2021-07-21T18:57:28Z

When our text buffer is full, newlines cause the buffer to scroll underneath the viewport (rather than the viewport moving down). This was causing selections made during text output to scroll down. To solve this, when we increment the circular buffer, we decrement the y-coordinates of the current selections by 1. We also invalidate the previous selection rects.

Closes #10319

src/renderer/base/renderer.cpp

DHowett · 2021-07-21T22:34:57Z

src/cascadia/TerminalCore/Terminal.cpp

+            // Update our selection too, so it doesn't move as the buffer is cycled
+            if (_selection)
+            {
+                _selection->start.Y -= 1;


What if the selection starts to go off the top of the screen (Y=0)?

Updated, if start.Y is at 0, we only reduce end.Y (if end.Y is greater than 0). If both are at 0, we clear the selection

Update: if we're only reducing end.Y, then we also set start.X = 0 to keep the block selection

…/pabhoj/scroll_select_fix

src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp

github-actions · 2021-08-02T16:56:07Z

@check-spelling-bot Report

Unrecognized words, please review:

Ouput

Previously acknowledged words that are now absent

SPACEBAR Unregister

To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:microsoft/terminal.git repository
on the dev/pabhoj/scroll_select_fix branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
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)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/4b45bb8df1f90e6fc463172cb861bf968e162e77.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/terminal/issues/comments/891179436" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u

✏️ 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/spelling/allow/names.txt.
... APIs, you can add them to a file in .github/actions/spelling/allow/.
... just things you're using, please add them to an appropriate file in .github/actions/spelling/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/spelling/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. 😉

🗜️ If you see a bunch of garbage

If it relates to a ...

well-formed pattern

See if there's a pattern that would match it.

If not, try writing one and adding it to a patterns/{file}.txt.

Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

Note that patterns can't match multiline strings.

binary-ish string

Please add a file path to the excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

…/pabhoj/scroll_select_fix

zadjii-msft · 2021-08-19T20:16:48Z

Any chance this also fixes #7524?

PankajBhojwani · 2021-08-19T20:30:52Z

Any chance this also fixes #7524?

I'm leaning towards no because this fix only touches what happens when we IncrementCircularBuffer and #7524 doesn't mention anything about the buffer needing to be full (tho it doesn't mention any other repro steps either so idk maybe?)

DHowett

Thanks!

ghost · 2021-08-20T22:36:07Z

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.

…fer is full (#10749) When our text buffer is full, newlines cause the buffer to scroll underneath the viewport (rather than the viewport moving down). This was causing selections made during text output to scroll down. To solve this, when we increment the circular buffer, we decrement the y-coordinates of the current selections by 1. We also invalidate the previous selection rects. Closes #10319

ghost · 2021-08-31T17:03:00Z

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

Handy links:

ghost · 2021-08-31T17:08:06Z

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

Handy links:

This builds upon #10749. When we added a separate pivot to track the "active" anchor, we forgot to update the pivot while circling. What does that mean? Moving the mouse would trigger us to update the selection using new endpoint and the old _pivot_, which hadn't been updated. There's probably a more elegant way of doing this, but it's good enough. Updated the test to cover this as well. Closes #14462

PankajBhojwani added 2 commits July 21, 2021 10:40

invalidate old selections

65f3c95

fix

0448533

ghost added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Jul 21, 2021

PankajBhojwani added 2 commits July 21, 2021 13:31

noexcept, mockrenderdata

e350ff2

noexcept

c5860fe

DHowett requested changes Jul 21, 2021

View reviewed changes

src/renderer/base/renderer.cpp Outdated Show resolved Hide resolved

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 21, 2021

don't go to renderer

e8ab28d

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 21, 2021

DHowett reviewed Jul 21, 2021

View reviewed changes

PankajBhojwani added 4 commits July 21, 2021 16:01

scroll off screen case

55e8802

start.X

2db3b6c

Merge branch 'main' of https://github.com/microsoft/terminal into dev…

72f2975

…/pabhoj/scroll_select_fix

add test

bb8831d

carlos-zamora approved these changes Jul 26, 2021

View reviewed changes

src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp Outdated Show resolved Hide resolved

src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp Show resolved Hide resolved

test fixes

daa4c0b

PankajBhojwani added 3 commits August 2, 2021 09:56

spell

e5df389

Merge branch 'main' of https://github.com/microsoft/terminal into dev…

c946f3e

…/pabhoj/scroll_select_fix

6th argument

36eaed8

zadjii-msft approved these changes Aug 20, 2021

View reviewed changes

DHowett approved these changes Aug 20, 2021

View reviewed changes

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 20, 2021

ghost merged commit cb2f347 into main Aug 20, 2021

ghost deleted the dev/pabhoj/scroll_select_fix branch August 20, 2021 22:36

zadjii-msft mentioned this pull request Aug 23, 2021

Text selection breaks in funny ways when program keeps printing to console #7524

Open

ghost mentioned this pull request Aug 31, 2021

Unable to select text correctly while new lines are printed and history buffer is full #10319

Closed

zadjii-msft mentioned this pull request Jan 5, 2023

Make sure to update the selection pivot while circling #14636

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix text selection while new lines are being printed when history buffer is full #10749

Fix text selection while new lines are being printed when history buffer is full #10749

PankajBhojwani commented Jul 21, 2021 •

edited by DHowett

Loading

DHowett Jul 21, 2021

PankajBhojwani Jul 21, 2021

PankajBhojwani Jul 21, 2021

github-actions bot commented Aug 2, 2021 •

edited

Loading

zadjii-msft commented Aug 19, 2021

PankajBhojwani commented Aug 19, 2021

DHowett left a comment

ghost commented Aug 20, 2021

ghost commented Aug 31, 2021

ghost commented Aug 31, 2021

Fix text selection while new lines are being printed when history buffer is full #10749

Fix text selection while new lines are being printed when history buffer is full #10749

Conversation

PankajBhojwani commented Jul 21, 2021 • edited by DHowett Loading

DHowett Jul 21, 2021

Choose a reason for hiding this comment

PankajBhojwani Jul 21, 2021

Choose a reason for hiding this comment

PankajBhojwani Jul 21, 2021

Choose a reason for hiding this comment

github-actions bot commented Aug 2, 2021 • edited Loading

@check-spelling-bot Report

Unrecognized words, please review:

zadjii-msft commented Aug 19, 2021

PankajBhojwani commented Aug 19, 2021

DHowett left a comment

Choose a reason for hiding this comment

ghost commented Aug 20, 2021

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 commented Aug 31, 2021

ghost commented Aug 31, 2021

PankajBhojwani commented Jul 21, 2021 •

edited by DHowett

Loading

github-actions bot commented Aug 2, 2021 •

edited

Loading

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.