Integrate base/ platform from VS Code and adopt scroll bar #5096

Tyriar · 2024-07-09T21:00:51Z

Last one I swear 🤞

This PR includes the base/ platform directory from VS Code and replaces our old Viewport with DomScrollableElement. This makes the scroll bar much nicer, not only in term of UX but also to the developer as it's much more reliable and flexible. The new project vs/ includes a subset of base/ that includes only files that are referenced and then has some files changed that weren't getting tree shaken correctly (eg. the grapheme related stuff).

Included are 2 important scripts that will help us update base/ when needed in the future to pull in bug fixes and/or new features:

bin/vs_base_update.ps1 will clone the repo, copy files over and do some clean up
bin/vs_base_find_unused.js uses the typescript compiler API to find any unused files for deletion before doing a commit

@jerch finishing up for the day but it would be great if you could give this a review now, in particular trying out the new scroll bar and look at how src/vs/ is integrated (don't bother reviewing src/vs/base/ as it's already had a lot of eyes on it). I think the only thing left to do on my end is to clean up the new Viewport usage and fix the TODOs. Planning on merging tomorrow if all goes well.

TODO:

Resolve todos
- Reimplement mouse events
Look into annoying ctrl+c issue where it doesn't go back to the bottom (not a regression)
Review Viewport implementation
Get CI passing
Record old/new core bundle sizes

jerch · 2024-07-10T10:22:24Z

@Tyriar Yupp most likely will be able to have a look at it in the afternoon. No clue yet if I manage to go deeper into details, ~20k loc is still a frightening number for a single quick review. I also have no doubt about code quality of the pulled in source, thus will skip most inner parts and mostly stick to a better conceptual comprehension.

One thing upfront - what you write about getting code fixes carried over with the help of additional scripts sounds a bit bothersome to me from a maintenance perspective. Do you see a chance, that your team would bundle&release the custom scrollbar functionality as a standalone package in the future? (Well idk if thats feasible or worth the trouble at all, have not yet looked at the code...)

Tyriar · 2024-07-10T12:50:57Z

One thing upfront - what you write about getting code fixes carried over with the help of additional scripts sounds a bit bothersome to me from a maintenance perspective.

I will probably be the one doing this and I doubt it will happen very often, like so infrequently that it definitely isn't worth trying to ship some lib. For example we will likely want to move over to the event and lifecycle code since it's more robust than ours and duplicates its functionality, and I think since those files are already present they won't need to bring in any changes.

This isn't used anymore, it was used earlier to workaround recursion between the viewport and the terminal objects

Tyriar · 2024-07-11T15:40:44Z

src/browser/Viewport.ts

+    this.register(coreMouseService.onProtocolChange(type => {
+      this._scrollableElement.updateOptions({
+        handleMouseWheel: !(type & CoreMouseEventType.WHEEL)
+      });
+    }));


@jerch this was easy and works great 😄

Yay, C-ish will safe the world someday, but before that, drive all programmers mad 😺

tisilent · 2024-07-11T15:56:16Z

Prelude to the feasibility of GPU rendering in code editor. 😃

Tyriar · 2024-07-11T16:19:10Z

Here's the output, there's a pretty significant increase for the main xterm.js package but I think we should accept it still because:

This will become the legacy package and maybe soon be replaced by a more efficient esbuild-based one.
We will be able to remove parts of xterm.js in favor of base/ parts like events.ts and lifecycle.ts.
For VS Code specifically the terminal startup is also deferred shortly to ensure we don't block the time to get an editor up and running so the impact there should be minimal.

All the addons are the same as expected so nothing leaked in there.

.js output (webpack) - measured by looking at Windows Explorer (seems to be larger? kB vs KiB?):

File	Before	After
xterm.js	285kb	475kb (+67%)
xterm-addon-attach.js	2kb	2kb
xterm-addon-canvas.js	93kb	93kb
xterm-addon-clipboard.js	7kb	7kb
xterm-addon-fit.js	2kb	2kb
xterm-addon-image.js	55kb	55kb
xterm-addon-ligatures.js	193kb	193kb
xterm-addon-search.js	12kb	12kb
xterm-addon-serialize.js	16kb	16kb
xterm-addon-unicode11.js	12kb	12kb
xterm-addon-unicode-graphemes.js	18kb	18kb
xterm-addon-web-links.js	4kb	4kb
xterm-addon-webgl.js	99kb	99kb

.mjs output (esbuild) - measured by esbuild output:

File	Before	After
xterm.mjs	252kb	333kb (+32%)
xterm-addon-attach.mjs	2kb	2kb
xterm-addon-canvas.mjs	82kb	82kb
xterm-addon-clipboard.mjs	5kb	5kb
xterm-addon-fit.mjs	2kb	2kb
xterm-addon-image.mjs	52kb	52kb
xterm-addon-ligatures.mjs	199kb	199kb
xterm-addon-search.mjs	11kb	11kb
xterm-addon-serialize.mjs	16kb	16kb
xterm-addon-unicode11.mjs	11kb	12kb
xterm-addon-unicode-graphemes.mjs	14kb	14kb
xterm-addon-web-links.mjs	3kb	3kb
xterm-addon-webgl.mjs	87kb	87kb

Tyriar · 2024-07-11T16:21:03Z

Planning on merging this soon today. I want to look briefly at the ctrl+c/smooth scroll issue that's been driving me crazy for months.

See microsoft/vscode#211199

Tyriar · 2024-07-11T16:42:45Z

Fixed microsoft/vscode#211199 finally! It was very difficult with the old viewport because of the mess of circular dependencies that was the scroll events

Fixes #211199 Fixes #216521 See xtermjs/xterm.js#5096

Tyriar · 2024-07-11T19:10:32Z

Maybe this will be a good opportunity to do a v6. While no big breaking changes it does:

Significantly change how the viewport works, so any custom styling of elements will break an embedder (like vscode)
fastScrollModifier now does nothing

tisilent · 2024-07-12T03:38:42Z

lib/xterm.mjs	332.6kb	100.0%
├ src/common/InputHandler.ts	43.5kb	13.1%
├ src/browser/CoreBrowserTerminal.ts	21.0kb	6.3%
├ src/browser/services/SelectionService.ts	12.6kb	3.8%
├ src/browser/renderer/dom/DomRenderer.ts	11.2kb	3.4%
├ src/vs/base/browser/ui/scrollbar/scrollableElement.ts	10.3kb	3.1%
├ src/vs/base/common/event.ts	9.6kb	2.9%
├ src/common/parser/EscapeSequenceParser.ts	9.5kb	2.9%
├ src/browser/AccessibilityManager.ts	8.2kb	2.5%
├ src/common/buffer/Buffer.ts	7.4kb	2.2%
├ src/browser/Linkifier.ts	6.6kb	2.0%
├ src/vs/base/common/scrollable.ts	5.8kb	1.7%
├ src/common/buffer/BufferLine.ts	5.6kb	1.7%
├ src/browser/renderer/dom/DomRendererRowFactory.ts	5.4kb	1.6%
├ src/vs/base/browser/dom.ts	5.3kb	1.6%
├ src/common/CoreTerminal.ts	4.9kb	1.5%
├ src/browser/services/RenderService.ts	4.9kb	1.5%
├ src/browser/decorations/OverviewRulerRenderer.ts	4.8kb	1.4%
├ src/vs/base/browser/touch.ts	4.7kb	1.4%
├ src/common/Color.ts	4.5kb	1.4%
├ src/browser/public/Terminal.ts	4.5kb	1.3%
├ src/vs/base/browser/ui/scrollbar/abstractScrollbar.ts	4.4kb	1.3%
├ src/vs/base/common/lifecycle.ts	4.2kb	1.3%
├ src/vs/base/browser/fastDomNode.ts	4.1kb	1.2%
├ src/browser/services/ThemeService.ts	4.0kb	1.2%
├ src/browser/Viewport.ts	3.9kb	1.2%
├ src/common/input/Keyboard.ts	3.8kb	1.1%
├ src/vs/base/common/async.ts	3.5kb	1.0%
├ src/common/buffer/AttributeData.ts	3.4kb	1.0%
├ src/browser/decorations/BufferDecorationRenderer.ts	3.4kb	1.0%
├ src/vs/base/common/hash.ts	3.2kb	1.0%
├ src/browser/input/CompositionHelper.ts	3.0kb	0.9%
├ src/common/services/OptionsService.ts	2.9kb	0.9%
├ src/common/services/CoreMouseService.ts	2.7kb	0.8%
├ src/vs/base/browser/ui/scrollbar/scrollbarState.ts	2.7kb	0.8%
├ src/common/SortedList.ts	2.7kb	0.8%
├ src/common/CircularList.ts	2.6kb	0.8%
├ src/common/input/UnicodeV6.ts	2.5kb	0.8%
├ src/browser/services/CharacterJoinerService.ts	2.5kb	0.7%
├ src/common/input/TextDecoder.ts	2.4kb	0.7%
├ src/common/parser/Params.ts	2.4kb	0.7%
├ src/common/parser/OscParser.ts	2.4kb	0.7%
├ src/browser/services/CoreBrowserService.ts	2.3kb	0.7%
├ src/common/parser/DcsParser.ts	2.2kb	0.7%
├ src/browser/services/CharSizeService.ts	2.2kb	0.7%
├ src/vs/base/common/platform.ts	2.1kb	0.6%
├ src/vs/base/browser/mouseEvent.ts	2.0kb	0.6%
├ src/browser/renderer/dom/WidthCache.ts	2.0kb	0.6%
├ src/common/services/DecorationService.ts	2.0kb	0.6%
├ src/common/input/WriteBuffer.ts	2.0kb	0.6%
├ src/vs/base/browser/browser.ts	1.7kb	0.5%
├ src/vs/base/browser/keyboardEvent.ts	1.7kb	0.5%
├ src/vs/base/browser/ui/scrollbar/scrollbarArrow.ts	1.7kb	0.5%
├ src/browser/selection/SelectionModel.ts	1.7kb	0.5%
├ src/browser/decorations/ColorZoneStore.ts	1.6kb	0.5%
├ src/vs/base/browser/ui/scrollbar/horizontalScrollbar.ts	1.5kb	0.5%
├ src/browser/input/MoveToCell.ts	1.5kb	0.5%
├ src/common/data/Charsets.ts	1.5kb	0.5%
├ src/common/buffer/BufferSet.ts	1.5kb	0.4%
├ src/vs/base/browser/ui/scrollbar/verticalScrollbar.ts	1.4kb	0.4%
├ src/common/services/BufferService.ts	1.4kb	0.4%
├ src/vs/base/common/linkedList.ts	1.4kb	0.4%
├ src/vs/base/common/iterator.ts	1.4kb	0.4%
├ src/common/buffer/BufferReflow.ts	1.4kb	0.4%
├ src/browser/renderer/shared/SelectionRenderModel.ts	1.3kb	0.4%
├ src/common/services/LogService.ts	1.3kb	0.4%
├ src/vs/base/common/errors.ts	1.3kb	0.4%
├ src/common/services/CoreService.ts	1.3kb	0.4%
├ src/common/TaskQueue.ts	1.2kb	0.4%
├ src/vs/base/browser/ui/scrollbar/scrollbarVisibilityController.ts	1.2kb	0.4%
├ src/browser/OscLinkProvider.ts	1.2kb	0.4%
├ src/common/services/UnicodeService.ts	1.2kb	0.4%
├ src/browser/RenderDebouncer.ts	1.2kb	0.4%
├ src/common/services/OscLinkService.ts	1.2kb	0.4%
├ src/vs/base/common/keybindings.ts	1.1kb	0.3%
├ src/browser/TimeBasedDebouncer.ts	1.1kb	0.3%
├ src/common/input/XParseColor.ts	968b	0.3%
├ src/vs/base/common/arrays.ts	942b	0.3%
├ src/common/buffer/CellData.ts	937b	0.3%
├ src/vs/base/common/cancellation.ts	917b	0.3%
├ src/common/services/InstantiationService.ts	867b	0.3%
├ src/vs/base/common/keyCodes.ts	846b	0.2%
├ src/common/data/EscapeSequences.ts	836b	0.2%
├ src/vs/base/browser/iframe.ts	817b	0.2%
├ src/vs/base/browser/globalPointerMoveMonitor.ts	815b	0.2%
├ src/browser/Clipboard.ts	785b	0.2%
├ src/browser/services/MouseService.ts	714b	0.2%
├ src/common/Lifecycle.ts	714b	0.2%
├ src/common/Platform.ts	711b	0.2%
├ src/common/public/BufferNamespaceApi.ts	708b	0.2%
├ src/vs/base/common/collections.ts	695b	0.2%
├ src/vs/base/browser/ui/widget.ts	651b	0.2%

Tyriar · 2024-07-12T13:40:21Z

@tisilent good to know 👍

jerch · 2024-07-15T13:36:38Z

@Tyriar Yay thx for getting this & the ESM stuff done! 🥳

While the impact on bundle size is quite steep, I couldn't care less, since we get some solid and well tested stuff in return...

Edit: Yes, I second the idea to do a new major release at this point. Even though the API did not change substantially, the changes overall are still quite a lot, and well - ESM alone would qualify for that from an outer perspective.

Tyriar added 8 commits July 9, 2024 10:41

Add git ignore for vs

190e1f0

Set up vs project

c30bffc

Get esbuild working and adopt vs scrollbar

4789c24

.

99db197

Helper script for finding unused files

86c875f

Delete unused files

c33ed46

Fix fit addon to get the scroll bar working in the demo

381200b

More clean up, doc updating vs/base

b401865

Tyriar added this to the 5.6.0 milestone Jul 9, 2024

Tyriar requested a review from jerch July 9, 2024 21:00

Tyriar self-assigned this Jul 9, 2024

Tyriar mentioned this pull request Jul 9, 2024

Include base/ platform from VS Code, adopt DomScrollableElement and introduce esbuild #5077

Closed

Tyriar added 11 commits July 10, 2024 06:22

Patch out unsafe usage of navigator and window in node envs

27195b0

Generate source maps so errors report ts files

d904f2f

Fix scroll lines API

1dada9f

Resolve vs imports in webpacked bundle

c3c787e

Remove unused demo webpack config

345b690

Exclude vs project from coverage

1b0078a

Hide more folders from explorer

522b39b

Tidy up esbuild bundle configs

8b83f44

Remove missing demo webpack npm script

c2b90c6

Remove ScrollSource

416f12f

This isn't used anymore, it was used earlier to workaround recursion between the viewport and the terminal objects

Have scroll bar bg react to theme change

75bf192

Tyriar mentioned this pull request Jul 10, 2024

Consider making the canvas selection further right to the overview ruler/scroll bar #5097

Open

Tyriar added 4 commits July 10, 2024 07:30

Support theming of overview ruler border

9b26bb4

Inline some functions

c7f8098

Support theming of scroll bar slider

884141c

Fix api lint

eca0800

Tyriar added 11 commits July 10, 2024 12:36

Fix api lint

d6f7133

Create shared file for viewport constants

1096b7f

_getMutableOptions -> _getChangeOptions

59a8f0c

Replace monaco- with xterm- in the update script

f155d67

Replace monaco- with xterm- in code/css files

444c07f

Fix out-esbuild build

31e0af7

Include all files, not just entry root folder

d532a15

Clean up dealing with animation frame

a644b5d

Fix mouse events

2da5eca

Implement zero scrollback+no mouse event wheel handling

721d483

Fix ICoreMouseService injecting

0f37351

Tyriar commented Jul 11, 2024

View reviewed changes

Resolve more todos

73ee951

Scroll immediately to bottom when scrollOnUserInput is enabled

90a1da2

See microsoft/vscode#211199

Tyriar enabled auto-merge July 11, 2024 16:36

Tyriar merged commit f3bfe80 into xtermjs:master Jul 11, 2024
12 checks passed

Tyriar added a commit to microsoft/vscode that referenced this pull request Jul 11, 2024

Update xterm and integrate new scroll bars

4f13098

Fixes #211199 Fixes #216521 See xtermjs/xterm.js#5096

This was referenced Jul 11, 2024

Decorations outside the scrollable area are not being updated #5101

Closed

Update xterm and integrate new scroll bars microsoft/vscode#221500

Merged

Tyriar added the breaking-change Breaks API and requires a major version bump label Jul 11, 2024

Tyriar modified the milestones: 5.6.0, 6.0.0 Jul 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate base/ platform from VS Code and adopt scroll bar #5096

Integrate base/ platform from VS Code and adopt scroll bar #5096

Tyriar commented Jul 9, 2024 •

edited

Loading

jerch commented Jul 10, 2024 •

edited

Loading

Tyriar commented Jul 10, 2024

Tyriar Jul 11, 2024

jerch Jul 15, 2024

tisilent commented Jul 11, 2024

Tyriar commented Jul 11, 2024

Tyriar commented Jul 11, 2024

Tyriar commented Jul 11, 2024

Tyriar commented Jul 11, 2024

tisilent commented Jul 12, 2024

Tyriar commented Jul 12, 2024

jerch commented Jul 15, 2024 •

edited

Loading

Integrate base/ platform from VS Code and adopt scroll bar #5096

Integrate base/ platform from VS Code and adopt scroll bar #5096

Conversation

Tyriar commented Jul 9, 2024 • edited Loading

jerch commented Jul 10, 2024 • edited Loading

Tyriar commented Jul 10, 2024

Tyriar Jul 11, 2024

Choose a reason for hiding this comment

jerch Jul 15, 2024

Choose a reason for hiding this comment

tisilent commented Jul 11, 2024

Tyriar commented Jul 11, 2024

Tyriar commented Jul 11, 2024

Tyriar commented Jul 11, 2024

Tyriar commented Jul 11, 2024

tisilent commented Jul 12, 2024

Tyriar commented Jul 12, 2024

jerch commented Jul 15, 2024 • edited Loading

Tyriar commented Jul 9, 2024 •

edited

Loading

jerch commented Jul 10, 2024 •

edited

Loading

jerch commented Jul 15, 2024 •

edited

Loading