Long Distance Hints #274700
Long Distance Hints #274700
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds Vite support for faster development builds and hot reload functionality to the VS Code project. It introduces infrastructure changes to enable running VS Code with Vite's dev server, including configuration files, build tooling updates, and environment variable support for dev mode.
Key Changes
- Added Vite and tsx dependencies for build tooling and TypeScript execution
- Introduced hot reload infrastructure and dev server configuration
- Added new rendering views for inline completions (long distance hints, layout utilities)
- Exposed new editor events (
onDidChangeViewZones) to support reactive view updates
Reviewed Changes
Copilot reviewed 36 out of 41 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | New Vite configuration with hot class support plugin |
| package.json | Added vite and tsx dependencies |
| src/main-js.js, src/bootstrap-fork.js | New entry points for tsx-based TypeScript execution |
| src/vs/editor/browser/observableCodeEditor.ts | Added view zones and line height observables for reactive updates |
| src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/ | New long-distance hint views and layout utilities |
| src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/inlineEditsModel.ts | Renamed InlineEditModel to ModelPerInlineEdit |
| src/vs/platform/windows/electron-main/windowImpl.ts | Added DEV_WINDOW_SRC environment variable support |
| src/vs/code/electron-browser/workbench/workbench.ts | Added Vite source root support |
| src/vs/base/browser/dom.ts | Moved hover observables from ObserverNodeWithElement to ObserverNode |
| /** | ||
| * Warning: This is not per inline edit id and gets created often. | ||
| */ | ||
| export class ModelPerInlineEdit implements ModelPerInlineEdit { |
There was a problem hiding this comment.
The class ModelPerInlineEdit implements itself, which is circular and incorrect. It should either implement an interface with a different name, or the implements clause should be removed if this is meant to be a concrete class without an interface.
| export class ModelPerInlineEdit implements ModelPerInlineEdit { | |
| export class ModelPerInlineEdit { |
vite.config.ts
Outdated
| //import { defineConfig } from 'vite'; | ||
|
|
||
| // use defineConfig({...}) | ||
| export default { |
There was a problem hiding this comment.
[nitpick] The commented-out import and instruction suggest using defineConfig, but the actual export doesn't use it. Either use defineConfig for better type safety and IDE support, or remove the comment to avoid confusion.
vite.config.ts
Outdated
| code = code + `\n | ||
| if (import.meta.hot) { | ||
| import.meta.hot.accept(); | ||
| }`; |
There was a problem hiding this comment.
The template string starts on line 37 but the content begins on line 38, creating an awkward line break. Use a proper template literal: code = code + `\\nif (import.meta.hot) {\\n\\timport.meta.hot.accept();\\n}`;
| /** | ||
| * The tower areas are arranged from left to right, touch and are aligned at the bottom. | ||
| * The requested tower is placed at the requested left offset. | ||
| */ |
There was a problem hiding this comment.
The documentation says 'touch and are aligned' which is grammatically awkward. Consider: 'The tower areas are arranged from left to right, touching each other and aligned at the bottom.'
| const layout = distributeFlexBoxLayout(1000, { | ||
| spaceBefore: { min: 10, max: 100, priority: 2, share: 1 }, | ||
| content: [{ min: 50, max: 100, priority: 2, share: 2 }, { min: 100, max: 500, priority: 1 }], | ||
| spaceAfter: {}, | ||
| }); |
There was a problem hiding this comment.
This appears to be leftover test/example code at the top level of the module. It should be removed or moved into a test file or commented out as an example.
| return new OffsetRange(55, 400); | ||
| return new OffsetRange(0, editor.getContentWidth()); |
There was a problem hiding this comment.
Line 426 has a hardcoded return value and line 427 is unreachable code. Remove line 426 to make the function return the actual content width.
src/vs/base/common/network.ts
Outdated
| * **Note:** use `dom.ts#asCSSUrl` whenever the URL is to be used in CSS context. | ||
| */ | ||
| asBrowserUri(resourcePath: AppResourcePath | ''): URI { | ||
| // For webworkers |
There was a problem hiding this comment.
The comment 'For webworkers' is vague. It should explain why this special case exists for files ending with 'Main.js' and what the purpose of using the SRC_ROOT is in this context.
| // For webworkers | |
| /** | |
| * Special handling for web worker entry points: | |
| * Files ending with 'Main.js' are typically entry scripts for web workers. | |
| * In certain environments (such as when running in a browser or with dynamic loading), | |
| * worker scripts must be loaded from a specific source root that may differ from the | |
| * standard resource resolution logic. The global variable `_VSCODE_SRC_ROOT` is set at runtime | |
| * to provide the correct base path for loading these worker scripts. | |
| * This ensures that worker entry points are resolved relative to the actual source root, | |
| * allowing them to be loaded correctly regardless of deployment or bundling configuration. | |
| */ |
src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/inlineEditsView.ts
Show resolved
Hide resolved
hediet
force-pushed
the
hediet/long-distance-hints
branch
from
d8b9527 to
3d182cf
Compare
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
|
No description provided.