Skip to content

Using computed editor options instead of direct values #300868

Merged
aiday-mar merged 3 commits intomainfrom
remaining-herring
Mar 11, 2026
Merged

Using computed editor options instead of direct values #300868
aiday-mar merged 3 commits intomainfrom
remaining-herring

Conversation

@aiday-mar
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings March 11, 2026 18:10
@aiday-mar aiday-mar self-assigned this Mar 11, 2026
@aiday-mar aiday-mar marked this pull request as ready for review March 11, 2026 18:12
@aiday-mar aiday-mar enabled auto-merge (squash) March 11, 2026 18:13
@aiday-mar aiday-mar marked this pull request as draft March 11, 2026 18:13
auto-merge was automatically disabled March 11, 2026 18:13

Pull request was converted to draft

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors editor line-wrapping / line-break computation to pass around computed editor options (IComputedEditorOptions) instead of individual option values, and updates affected unit tests and view model plumbing accordingly.

Changes:

  • Change ILineBreaksComputerFactory.createLineBreaksComputer to accept IComputedEditorOptions + tabSize (instead of fontInfo/wrapping parameters).
  • Update view model line wrapping (ViewModelLinesFromProjectedModel) and configuration-change handling to forward computed options.
  • Update related browser/common line break computers and tests to use the new API shape.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/vs/editor/test/browser/viewModel/monospaceLineBreaksComputer.test.ts Updates test setup to construct ComputedEditorOptions for the new factory signature.
src/vs/editor/test/browser/viewModel/modelLineProjection.test.ts Updates ViewModelLinesFromProjectedModel construction to pass computed options.
src/vs/editor/common/viewModel/viewModelLines.ts Refactors wrapping settings handling to use ConfigurationChangedEvent + computed options.
src/vs/editor/common/viewModel/viewModelImpl.ts Updates view model construction and config-change path to pass computed options through.
src/vs/editor/common/viewModel/monospaceLineBreaksComputer.ts Refactors monospace line break computation to read needed values from computed options.
src/vs/editor/common/modelLineProjectionData.ts Updates the ILineBreaksComputerFactory interface to the new signature.
src/vs/editor/browser/view/domLineBreaksComputer.ts Refactors DOM-based line break computation to read needed values from computed options.
Comments suppressed due to low confidence (1)

src/vs/editor/test/browser/viewModel/monospaceLineBreaksComputer.test.ts:80

  • This test now sets EditorOption.wordWrapColumn, but the line breaking code should be driven by the effective wrapping column (EditorOption.wrappingInfo.wrappingColumn). Once the production code is corrected to use wrappingInfo, update the test to write a wrappingInfo value (with wrappingColumn: breakAfter) instead of setting wordWrapColumn directly.

@aiday-mar aiday-mar marked this pull request as ready for review March 11, 2026 21:43
@aiday-mar aiday-mar enabled auto-merge (squash) March 11, 2026 21:43
@vs-code-engineering vs-code-engineering bot added this to the 1.112.0 milestone Mar 11, 2026
@aiday-mar aiday-mar merged commit 446c755 into main Mar 11, 2026
20 checks passed
@aiday-mar aiday-mar deleted the remaining-herring branch March 11, 2026 21:50
justschen added a commit that referenced this pull request Mar 12, 2026
justschen added a commit that referenced this pull request Mar 12, 2026
justschen added a commit that referenced this pull request Mar 12, 2026
…) (#301023)

Revert "Using computed editor options instead of direct values  (#300868)"

This reverts commit 446c755.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants