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

Option to cause window to automatically scroll to bottom when new output is received #8879

Open
richardthombs opened this issue Jan 25, 2021 · 3 comments
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone

Comments

@richardthombs
Copy link

richardthombs commented Jan 25, 2021

Description of the new feature/enhancement

Add a profileoOption to cause window to automatically scroll to bottom when new output is received

When I am working on ASP.NET with Entity Framework, I have a dotnet watch run pane active.

EF Core is particularly verbose with its stack traces, so often the root error message is so far up the trace that I have to scroll then window up to see it.

Once I've fixed the error and recompiled, it would be great if the pane auto-scrolled back to the bottom so that I can see the output from the current compilation. As it stands at the moment, I have to scroll back down manually each time, which is slow and often I forget and think that I'm looking at a new error when in actual fact I'm still staring at the old one.

Proposed technical implementation details (optional)

I would like to be able to set a flag on panes like this so that they always scroll the most recent output into view.

@richardthombs richardthombs added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Jan 25, 2021
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jan 25, 2021
@zadjii-msft
Copy link
Member

Alright, well color me surprised. We discussed making this a setting in #2529, and implemented in #6062, also related are #980 and #3863.

Looks like we decided against making this a setting, because

  1. No other terminal emulators had a similar setting
  2. The user can just press a key and leverage the snapOnInput behavior to snap to the bottom
  3. We couldn't find a good way of expressing a setting for this

So I guess now that there's a request for a setting, do we need to re-open that discussion?

@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Jan 25, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 25, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Jan 25, 2021
@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jan 26, 2021
@richardthombs
Copy link
Author

@zadjii-msft Thanks for considering this. I would say that your reason 2 doesn't reflect all the actions that my particular scenario requires as a workaround. I have a multi-monitor setup with this terminal window on a secondary display. So I have to:

  1. Move mouse over to the window & pane.
  2. Click the mouse.
  3. Hit a key.
  4. Alt-Tab back to VS Code.
  5. Move mouse across monitors and back into the VS Code window.

1 & 2 could be done with Alt-Tab, but there is no telling how deep the required window will be in the stack so there is a bit of a visual recognition delay while hunting for the correct image to Tab, Tab, Tab to.

@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
ghost pushed a commit that referenced this issue Apr 14, 2022
This is an attempt to simplify the `ConGetSet` interface down to the
smallest set of methods necessary to support the `AdaptDispatch` class.
The idea is that it should then be easier to implement that interface in
Windows Terminal, so we can replace the `TerminalDispatch` class with
`AdaptDispatch`.

This is a continuation of the refactoring started in #12247, and a
significant step towards #3849.

## Detailed Description of the Pull Request / Additional comments

The general idea was to give the `AdaptDispatch` class direct access to
the high-level structures on which it needs to operate. Some of these
structures are now passed in when the class is constructed (the
`Renderer`, `RenderSettings`, and `TerminalInput`), and some are exposed
as new methods in `ConGetSet` (`GetStateMachine`, `GetTextBuffer`, and
`GetViewport`).

Many of the existing `ConhostInternalGetSet` methods could easily then
be reimplemented in `AdaptDispatch`, since they were often simply
forwarding to methods in one of the above structures. Some were a little
more complicated, though, and require further explanation.

* `GetConsoleScreenBufferInfoEx`: What we were typically using this for
  was to obtain the viewport, although what we really wanted was the
  virtual viewport, which is now accessible via the `GetViewport`
  method. This was also used to obtain the cursor position and buffer
  width, which we can now get via the `GetTextBuffer` method.

* `SetConsoleScreenBufferInfoEx`: This was only really used for the
  `AdaptDispatch::SetColumns` implementation (for `DECCOLM` mode), and
  that could be replaced with `ResizeWindow`. This is a slight change in
  behaviour (it sizes the window rather than the buffer), but neither is
  technically correct for `DECCOLM`, so I think it's good enough for
  now, and at least it's consistent with the other VT sizing operations.

* `SetCursorPosition`: This could mostly be replaced with direct
  manipulation of the `Cursor` object (accessible via the text buffer),
  although again this is a slight change in behavior. The original code
  would also have made a call to `ConsoleImeResizeCompStrView` (which I
  don't think is applicable to VT movement), and would potentially have
  moved the viewport (not essential for now, but could later be
  supported by `DECHCCM`). It also called `VtIo::SetCursorPosition` to
  handle cursor inheritance, but that should only apply to
  `InteractDispatch`, so I've moved that to the
  `InteractDispatch::MoveCursor` method.

* `ScrollRegion`: This has been replaced by two simple helper methods in
  `AdaptDispatch` which better meet the VT requirements -
  `_ScrollRectVertically` and `_ScrollRectHorizontally`. Unlike the
  original `ScrollRegion` implementation, these don't generate
  `EVENT_CONSOLE_UPDATE_SCROLL` events (see #12656 for more details).

* `FillRegion`: This has been replaced by the `_FillRect` helper method
  in `AdaptDispatch`. It differs from the original `FillRegion` in that
  it takes a rect rather than a start position and length, which gives
  us more flexibility for future operations.

* `ReverseLineFeed`: This has been replaced with a somewhat refactored
  reimplementation in `AdaptDispatch`, mostly using the
  `_ScrollRectVertically` helper described above.

* `EraseAll`: This was previously handled by
  `SCREEN_INFORMATION::VtEraseAll`, but has now been entirely
  reimplemented in the `AdaptDispatch::_EraseAll` method.

* `DeleteLines`/`InsertLines`/`_modifyLines`: These have been replaced
  by the `_InsertDeleteLineHelper` method in `AdaptDispatch`, which
  mostly relies on the `_ScrollRectVertically` helper described above.

Finally there were a few methods that weren't actually needed in the
`ConGetSet` interface:

* `MoveToBottom`: This was really just a hack to get the virtual
  viewport from `GetConsoleScreenBufferInfoEx`. We may still want
  something like in the future (e.g. to support `DECVCCM` or #8879), but
  I don't think it's essential for now.

* `SuppressResizeRepaint`: This was only needed in `InteractDispatch`
  and `PtySignalInputThread`, and they could easily access the `VtIo`
  object to implement it themselves.

* `ClearBuffer`: This was only used in `PtySignalInputThread`, and that
  could easily access the buffer directly via the global console
  information.

* `WriteControlInput`: This was only used in `InteractDispatch`, and
  that could easily be replaced with a direct call to
  `HandleGenericKeyEvent`.

As part of these changes, I've also refactored some of the existing
`AdaptDispatch` code:

* `_InsertDeleteHelper` (renamed `_InsertDeleteCharacterHelper`) is now
  just a straightforward call to the new `_ScrollRectHorizontally`
  helper.

* `EraseInDisplay` and `EraseInLine` have been implemented as a series
  of `_FillRect` calls, so `_EraseSingleLineHelper` is no longer
  required.

* `_EraseScrollback` is a essentially a special form of scrolling
  operation, which mostly depends on the `TextBuffer::ScrollRows`
  method, and with the filling now provided by the new `_FillRect`
  helper.

* There are quite a few operations now in `AdaptDispatch` that are
  affected by the scrolling margins, so I've pulled out the common
  margin setup into a new `_GetVerticalMargins` helper method. This also
  fixes some edge cases where margins could end up out of range.

## Validation Steps Performed
There were a number of unit tests that needed to be updated to work
around functions that have now been removed, but these substitutions
were fairly straightforward for the most part.

The adapter tests were a different story, though. In that case we were
explicitly testing how operations were passed through to the `ConGetSet`
interface, but with more than half those methods now gone, a significant
rewrite was required.

I've tried to retain the crux of the original tests, but we now have to
validate the state changes on the underlying data structures, where
before that state would have been tracked in the `TestGetSet` mock. And
in some cases we were testing what happened when a method failed, but
since that scenario is no longer possible, I've simply removed those
tests.

I've also tried to manually test all the affected operations to confirm
that they're still working as expected, both in vttest as well as my own
test scripts.

Closes #12662
@FlukeFan
Copy link

Having recently moved to using Windows Terminal, I'm finding the same issue (that it doesn't play nicely with "dotnet watch", and long stack traces where I need to scroll up to see what failed).

So I guess now that there's a request for a setting, do we need to re-open that discussion?

Has the discussion be re-opened?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

4 participants