-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Enable marks for Stable builds #15057
Comments
Alrighty so I don't know why Attempt 1 is failing for you. It's very possible that and my profiles are set up like: "profiles":
{
"defaults":
{
"experimental.autoMarkPrompts": true,
"experimental.showMarksOnScrollbar": true,
}, RE: Attempt 2: That's probably demoing RE: Attempt 3: That really makes me think that |
I'll add: I'm using PROMPT $e]133;D$e\$e]133;A$e\$e]9;9;$P$e\$e[107;30m$T]$e[97;46m$G$P$e[36;49m$G$e[0m$_$e[0m$e[94m%username%$e[0m@$e[32m%computername%$e[0m$G$e]133;B$e as my CMD prompt, and |
Thanks @zadjii-msft. Settings were / still are under the Using your CMD prompt, I still don't get marks: And here's
|
Oh my god I totally know what this is. We never enabled that feature for Stable builds. It's still in Preview-only builds. Lines 130 to 139 in 34aa6aa
There's still some edge cases that need to be worked out before I'd feel comfortable enabling them for the general public. Would you mind if I repurpose this thread as the "Enable marks for Stable Builds ship list"? |
Hah so you thought this was in stable but it wasn't? My thinking was it was announced in 1.15 preview (https://devblogs.microsoft.com/commandline/windows-terminal-preview-1-15-release/#scroll-marks-experimental) so it should definitely be there in 1.16, but I guess that's not how it works. 🤔 And sure repurpose the thread, the cause is "marks aren't enabled for stable ship list" so sure that's a good description. |
Yea, there's certain features we want to check in to |
Cool. One suggestion to help people use the new feature is to include the exact terminal sequences we need for pwsh, bash, and cmd in the release notes (so people can copy and paste). |
Move scroll marks to `TextBuffer`, so they can be cleared by EraseInDisplay and EraseScrollback. Also removes the namespacing on them. ## References and Relevant Issues * see also #11000 and #15057 * Resize/Reflow _doesn't_ work yet and I'm not attempting this here. ## Validation Steps Performed * `cls` works * `Clear-Host` works * `clear` works * the "Clear buffer" action works * They work when there's marks above the current viewport, and clear the scrollback * they work if you clear multiple "pages" of output, then scroll back to where marks previously were * resizing doesn't totally destroy the marks Closes #15426
Reflow notes:
and when we start a prompt, we create prompt data on that row. We're never (in practice) going to have two separate prompts on the same row. We only need one scroll mark per row. that gives us
Unfiltered Teams notes
addenda: I'm definitely going to just have the text buffer also be able to hand back a ScrollMark{...} comprised of the same 4 points as before. That way the buffer can store that all as attrs and all that, but then the control layer can just be like "yo give me the relevant regions of text for this prompt" |
Won’t that destroy the prompt start and prompt end locations, which we use for some of our detection features? Or would we figure those out after the fact by checking out where Prompt ends and Commans begins? |
@DHowett I was thinking that the PromptData on the Row is just like, a bookmark. "On this row, a prompt starts". Then we iterate over the attr runs themselves (possibly wrapping if need be) to gather up the actual text of the prompt. We honestly could just iterate over the attr runs directly to do this, but the PromptData is there then to stash away the exit code (and color, or the literal command text, or what have you), without us needing to stick it into the TextAttributes themselves. (we can also extend |
* Switches the marks feature to being stable. * Renames the settings, to remove "experimental." Closes #15057
This is pretty much a huge refactoring of how marks are stored in the buffer. Gone is the list of `ScrollMark`s in the buffer that store regions of text as points marking the ends. Those would be nigh impossible to reflow nicely. Instead, we're going to use `TextAttribute`s to store the kind of output we've got - `Prompt`, `Command`, `Output`, or, the default, `None`. Those already reflow nicely! But we also need to store things like, the exit code for the command. That's why we've now added `ScrollbarData` to `ROW`s. There's really only going to be one prompt->output on a single row. So, we only need to store one ScrollbarData per-row. When a command ends, we can just go update the mark on the row that started that command. But iterating over the whole buffer to find the next/previous prompt/command/output region sounds complicated. So, to avoid everyone needing to do some variant of that, we've added `MarkExtents` (which is literally just the same mark structure as before). TextBuffer can figure out where all the mark regions are, and hand that back to callers. This allows ControlCore to be basically unchanged. _But collecting up all the regions for all the marks sounds expensive! We need to update the scrollbar frequently, we can't just collect those up every time!_ No we can't! But we also don't need to. The scrollbar doesn't need to know where all the marks start and end and if they have commands and this and that - no. We only need to know the rows that have marks on them. So, we've now also got `ScrollMark` to represent just a mark on a scrollbar at a specific row on the buffer. We can get those quickly. * [x] I added a bunch of tests for this. * [x] I played with it and it feels good, even after a reflow (finally) * See: * #11000 * #15057 (I'm not marking this as closed. The stacked PR will close this, when I move marks to Stable)
maintainer note: hijacking thread to track bugs we need to fix before enabling marks for Stable builds
See also: #11000
Tasks
autoMarkPrompts
#15813Windows Terminal version
1.16.10262.0
Windows build number
10.0.22621.0
Other Software
I am attempting to add a scroll mark, which I believe should appear as a notch on the scroll bar, per #12948.
No method to create a scroll mark results in a visible notch in the scroll bar.
I have
"experimental.showMarksOnScrollbar": true
enabled on my profileSteps to reproduce
Attempt 1 - echo (what I want so I can make my prompt auto-mark)
The docs have a screenshot of the relevant text rather than characters:
I had to OCR the above image, and I've checked it a couple of times after @ / 0 being switched, but I'm relatively sure it is:
I have
"experimental.showMarksOnScrollbar": true
enabled on my profile but it doesn't seem to work. I expected a mark on the scroll bar. to appear after running this command.Attempt 2 - do... something I don't understand
There is also this animated GIF (see docs for animation).
I do not understand how
echo "mark 1"
is making a mark. The docs mention there is anaddMark
action but I do not understand howmark 1
andmark 2
etc are related to theaddMark
action.Run 'Add Mark' action manually.
I have tried running the AddMark action manually as well:
again I can't get marks to appear. Windows Terminal 1.16.10262.0.
Thanks for understanding - I'm really excited to try this feature, and I appreciate you creating it. ❤️
Expected Behavior
Get a small notch on the scroll bar
Actual Behavior
The scroll bar remains unchanged.
The text was updated successfully, but these errors were encountered: