Skip to content

Fix/scrollbar marks shell integration #19185

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

Merged

Conversation

killerdevildog
Copy link
Contributor

Fix scrollbar marks not showing with shell integration
Fixes #19104

This PR resolves an issue where scrollbar marks created by shell integration sequences (OSC 133 FTCS, OSC 1337 iTerm2, and OSC 9;12 ConEmu sequences) were not visible on the scrollbar until the user manually scrolled. The problem was that while marks were being created in the buffer correctly, the UI wasn't being notified to refresh the scrollbar display. The fix adds a new NotifyShellIntegrationMark() method to the ITerminalApi interface that calls _NotifyScrollEvent() to trigger scrollbar refresh, and updates all shell integration sequence handlers in AdaptDispatch to call this notification method after creating marks. This ensures scrollbar marks appear immediately when shell integration sequences are processed, bringing feature parity between auto-detected and shell-integration-based marks.

PR Checklist

- Add NotifyShellIntegrationMark() method to ITerminalApi interface
- Implement the method in Terminal to call _NotifyScrollEvent()
- Call the notification method from AdaptDispatch::DoFinalTermAction()
  when FTCS sequences (OSC 133 A/B/C/D) add marks to the buffer

This fixes the issue where scrollbar marks created by shell integration
(autoMarkPrompts) would not appear until the first scroll event occurred.
The marks were being created in the buffer but the scrollbar UI was not
being invalidated/refreshed.

Fixes microsoft#19104
- Add NotifyShellIntegrationMark() method declaration to outputStream.hpp
- Implement empty method in outputStream.cpp with appropriate comment
- Fixes abstract class compilation error for ITerminalApi interface
- Shell integration marks are a Terminal app feature, not implemented in conhost
- Implement missing method in unit test mock class
- Ensures all ITerminalApi implementations are complete
- Required for compilation after adding method to interface
- Add notification to OSC1337;SetMark (iTerm2 shell integration)
- Add notification to OSC9;12 (ConEmu shell integration)
- Ensures all shell integration mark creation methods trigger scrollbar refresh
- Maintains consistency with FTCS sequence handlers
void Terminal::NotifyShellIntegrationMark()
{
// Notify the scrollbar that marks have been added so it can refresh the mark indicators
_NotifyScrollEvent();
Copy link
Member

Choose a reason for hiding this comment

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

(To the terminal team:) The synchronous coupling of the output thread --> buffer --> control is a continuously increasing issue IMO. Not just from a perf. standpoint where these callbacks trigger logic while holding the global lock, but also due to us having to painstakingly add logic like this. = Development and runtime overhead. It should be tackled as a proper task in the near-term IMO.

Copy link
Member

Choose a reason for hiding this comment

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

heard. would love to do something about it - without stopping to rewrite the world :)

@lhecker
Copy link
Member

lhecker commented Jul 29, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lhecker lhecker requested review from DHowett and zadjii-msft July 29, 2025 15:07
@DHowett
Copy link
Member

DHowett commented Jul 29, 2025

Thanks so much for fixing this!

@github-project-automation github-project-automation bot moved this to To Cherry Pick in 1.23 Servicing Pipeline Jul 29, 2025
@DHowett DHowett merged commit e818daf into microsoft:main Jul 29, 2025
15 checks passed
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.23 Servicing Pipeline Jul 29, 2025
DHowett pushed a commit that referenced this pull request Jul 29, 2025
This PR resolves an issue where scrollbar marks created by shell
integration sequences (OSC 133 FTCS, OSC 1337 iTerm2, and OSC 9;12
ConEmu sequences) were not visible on the scrollbar until the user
manually scrolled.

The problem was that while marks were being created in the buffer
correctly, the UI wasn't being notified to refresh the scrollbar
display. The fix adds a new NotifyShellIntegrationMark() method to the
ITerminalApi interface that calls _NotifyScrollEvent() to trigger
scrollbar refresh, and updates all shell integration sequence handlers
in AdaptDispatch to call this notification method after creating marks.
This ensures scrollbar marks appear immediately when shell integration
sequences are processed, bringing feature parity between auto-detected
and shell-integration-based marks.

Closes #19104

(cherry picked from commit e818daf)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgdFukU
Service-Version: 1.23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Cherry Picked
Development

Successfully merging this pull request may close these issues.

Scrollbar is not showing "marks" until first-scroll-event happens if the profile has shell-integration
3 participants