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

Experimental: add support for scrollbar marks #12948

Merged
merged 33 commits into from
Jun 9, 2022

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Apr 20, 2022

Adds support for marks in the scrollbar. These marks can be added in 3
ways:

  • Via the iterm2 OSC 1337 ; SetMark sequence
  • Via the addMark action
  • Automatically when the experimental.autoMarkPrompts per-profile
    setting is enabled.

#11000 has more tracking for the big-picture for this feature, as well
as additional follow-ups. This set of functionality seemed complete
enough to send a review for now. That issue describes these how I wish
these actions to look in the fullness of time. This is simply the v0.1
from the hackathon last month.

Actions

  • addMark: add a mark to the buffer. If there's a selection, use
    place the mark covering at the selection. Otherwise, place the mark
    on the cursor row.
    • color: a color for the scrollbar mark. This is optional - defaults
      to the foreground color of the current scheme if omitted.
  • scrollToMark
    • direction: ["first", "previous", "next", "last"]
  • clearMark: Clears marks at the current postition (either the
    selection if there is one, or the cursor position.
  • clearAllMarks: Don't think this needs explanation.

Per-profile settings

  • experimental.autoMarkPrompts: bool, default false.
  • experimental.showMarksOnScrollbar: bool

PR Checklist

Detailed Description of the Pull Request / Additional comments

This is basically hackathon code. It's experimental! That's okay! We'll
figure the rest of the design in post.

Theoretically, I should make these actions experimental. as well, but
it seemed like since the only way to see these guys was via the
experimental.showMarksOnScrollbar setting, you've already broken
yourself into experimental jail, and you know what you're doing.

Things that won't work as expected:

  • resizing, ESPECIALLY reflowing
  • Clearing the buffer with ED sequences / Clear Buffer

I could theoretically add velocity around this in the TermControl
layer. Always prevent marks from being visible, ignore all the actions.
Marks could still be set by VT and automark, but they'd be useless.

Next up priorities:

  • Making this work with the FinalTerm sequences
  • properly speccing
  • adding support for showMarksOnScrollbar: flags(categories), so you
    can only display errors on the scrollbar
  • adding the category flag to the addMark action

Validation Steps Performed

I like using it quite a bit. The marks can get noisy if you have them
emitted on every prompt and the buffer has 9000 lines. But that's the
beautiful thing, the actions work even if the marks aren't visible, so
you can still scroll between prompts.

Settings blob
// actions
        { "keys": "ctrl+up", "command": { "action": "scrollToMark", "direction": "previous" }, "name": "Previous mark" },
        { "keys": "ctrl+down", "command": { "action": "scrollToMark", "direction": "next" }, "name": "Next mark" },
        { "keys": "ctrl+pgup", "command": { "action": "scrollToMark", "direction": "first" }, "name": "First mark" },
        { "keys": "ctrl+pgdn", "command": { "action": "scrollToMark", "direction": "last" }, "name": "Last mark" },
        { "command": { "action": "addMark" } },
        { "command": { "action": "addMark", "color": "#ff00ff" } },
        { "command": { "action": "addMark", "color": "#0000ff" } },
        { "command": { "action": "clearAllMarks" } },

// profiles.defaults
        "experimental.autoMarkPrompts": true,
        "experimental.showMarksOnScrollbar": true,

@ghost ghost added Area-Extensibility A feature that would ideally be fulfilled by us having an extension model. Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels Apr 20, 2022
@github-actions

This comment was marked as resolved.

@zadjii-msft
Copy link
Member Author

zadjii-msft commented Apr 20, 2022

Dustin likes to keep the gifs out of the commit body:

gh-12948-marks-change-colors
fig 1: marks without a color change to match the default fg color

gh-12948-automark
fig 2: any time you press enter, a new mark is added

gh-12948-clear-mark
fig 3: you can add bookmarks at a selection, and clear them with a selection as well

gh-12948-scroll-between-marks
fig 4: scrolling between marks

many-marks-git-log
fig 5: marks on the scrollbar, during a git log

Copy link
Collaborator

@j4james j4james left a comment

Choose a reason for hiding this comment

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

I haven't reviewed in full - I was just looking at the dispatch code because I wanted to make sure it was going to work with the upcoming architectural changes.

src/cascadia/TerminalCore/TerminalDispatch.cpp Outdated Show resolved Hide resolved
src/host/outputStream.cpp Outdated Show resolved Hide resolved
src/terminal/adapter/ITermDispatch.hpp Outdated Show resolved Hide resolved
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 6, 2022
@zadjii-msft
Copy link
Member Author

  • Alt buffer doesn't display marks or accept them
  • It's behind velocity now

@zadjii-msft zadjii-msft merged commit 799b5d4 into main Jun 9, 2022
@zadjii-msft zadjii-msft deleted the dev/migrie/f/1527-experimental-marks branch June 9, 2022 21:10
ghost pushed a commit that referenced this pull request Jun 9, 2022
#13163)

Implements the **FTCS_PROMPT** sequence, `OSC 133 ; A ST`. In this PR, it's just used to set a simple Prompt mark on the current line, in the same way that the iTerm2 sequence works.

There's rumination in #11000 on how to implement the rest of the FTCS sequences. 

This is broken into its own PR at the moment. [Quoth j4james](#12948 (comment)):

> That should be just as easy, and I've noticed a couple of other terminals that are doing that, so it's not unprecedented. If we don't have any immediate use for the other options, there shouldn't be any harm in ignoring them initially.
> 
> And the benefit of going with the more widely supported sequence is that we're more likely to benefit from any shells that have this functionality built in. Otherwise they're forced to try and detect the terminal, which is practically impossible for Windows Terminal. Even iTerm2 supports the `OSC 133` sequence, so we'd probably be the only odd one out.

This part of the plumbing is super easy, so I thought it would be valuable to add regardless if we get to the whole of FTCS in 1.15.

* [x] I work here
* [x] Tested manually - in my pwsh `$PROFILE`:
  ```pwsh
  function prompt {
    $loc = $($executionContext.SessionState.Path.CurrentLocation);
    $out = "PS $loc$('>' * ($nestedPromptLevel + 1)) ";
    $out += "$([char]27)]9;9;`"$loc`"$([char]07)";
    $out += "$([char]27)]133;A;$([char]7)"; # add the FTCS_PROMPT to the... well, end, but you get the point
    return $out
  }
  ```
* See also #11000
ghost pushed a commit that referenced this pull request Jun 16, 2022
When I moved this into ControlCore, I forgot that UserScrollViewport is usually triggered by the scrollbar updating, so it doesn't ask the UI to update. Since this logic is in ControlCore, it's sorta in a weird place where it needs to communicate both up and down:
* update the `Terminal`'s viewport position
* update the `TermControl`'s scrollbar position

Checklist:

* [x] Closes a bug bash bug
* [x] Missed in #12948 
* See also #11000
carlos-zamora pushed a commit that referenced this pull request Jul 1, 2022
Update schema with the settings/actions added in #12948 

## PR Checklist
* [x] Closes #13404
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Schema updated.
DHowett pushed a commit that referenced this pull request Jul 5, 2022
Update schema with the settings/actions added in #12948

## PR Checklist
* [x] Closes #13404
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Schema updated.

(cherry picked from commit 478c2c3)
Service-Card-Id: 83790285
Service-Version: 1.15
@ghost
Copy link

ghost commented Jul 6, 2022

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

Handy links:

@mikemaccana
Copy link
Contributor

mikemaccana commented Mar 23, 2023

Link above is now a 404, so: https://devblogs.microsoft.com/commandline/windows-terminal-preview-1-15-release/#scroll-marks-experimental

(have moved comments re: not being able to make scroll marks to a separate issue linked below).

@i5513
Copy link

i5513 commented May 16, 2023

Sorry I was using terminal stable, in preview it is ok by default, thank you!

Would be useful if autoMarkPrompts has a second option (autoMarkIncludePrompt for example) to set the mark at the same line of the command, so you can see the command and the output each time you go to next / previous mark

If you are presenting a module of powershell it would be a very useful feature, I think it is not difficult to add this case use

Thank you very much!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Extensibility A feature that would ideally be fulfilled by us having an extension model. Area-Input Related to input processing (key presses, mouse, etc.) Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scroll to previous/next prompt FR: IDE-style marks on scrollbar
8 participants