Skip to content

Namespaced conceal range-clear (ConcealManager::remove_in_range_for_namespace)#2399

Merged
sinelaw merged 2 commits into
sinelaw:masterfrom
Agrejus:agrejus/core-decoration-primitives
Jun 24, 2026
Merged

Namespaced conceal range-clear (ConcealManager::remove_in_range_for_namespace)#2399
sinelaw merged 2 commits into
sinelaw:masterfrom
Agrejus:agrejus/core-decoration-primitives

Conversation

@Agrejus

@Agrejus Agrejus commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

What this is

One small, additive core primitive, carved out of #2325 so it can be reviewed and merged on its own. No graphics / kitty-protocol code is in here.

ConcealManager::remove_in_range_for_namespace()

Clears only one namespace's conceal ranges that overlap a byte range, leaving other plugins' conceals in that range untouched. This mirrors the existing OverlayManager::remove_in_range_for_namespace (issue #2146) — conceals just never had the namespaced variant.

It's exposed to plugins as editor.clearConcealsInRangeForNamespace(bufferId, namespace, start, end) via a new PluginCommand::ClearConcealsInRangeForNamespace. This lets a plugin rebuild its own conceals for a line (the common per-line lines_changed rebuild pattern) without stomping conceals another plugin placed on the same line.

Why it's split out like this

This is one of several PRs breaking up #2325, which I'm closing in favor of these smaller PRs.

This PR was originally scoped to also include a deterministic virtual-line ordering tiebreak in virtual_text.rs. Per @sinelaw's request, that change has been removed so the ConcealManager primitive can be merged on its own. The underlying scroll/flicker issue it addressed isn't reproducible on the maintainer's setup, so it'll be revisited separately rather than holding up this merge.

Risk

Low. The change is purely additive — the conceal method is brand new; nothing previously called it.

Tests

  • view::conceal::tests — namespace isolation (only the target namespace's in-range conceals are removed; other namespaces and out-of-range entries survive) plus range/version no-op semantics.

Full local CI passes (fmt, clippy, doc, schema, check --no-default-features, and the touched test modules). The ts_export API-surface tests (incl. the generated .d.ts validation) pass with the new method.


Supersedes #2325 (being closed). Follow-up PRs will cover the virtual-line ordering, the markdown-compose rendering, and, separately, the graphics/kitty work for review.

@sinelaw

sinelaw commented Jun 19, 2026

Copy link
Copy Markdown
Owner

What's the observable bug that is fixed by "Deterministic virtual-line ordering" ? Did you encounter an issue while building the graphics pr?

@Agrejus

Agrejus commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

What's the observable bug that is fixed by "Deterministic virtual-line ordering" ? Did you encounter an issue while building the graphics pr?

@sinelaw when I was viewing the markdown preview, scrolling vertically caused the pipe table borders in continuation rows to jump out of position. They would snap back into the correct position as I scrolled farther.

Turns out the continuation lines for a wrapped row are virtual texts sharing the same anchor and priority. They live in a HashMap, and the old sort only went position → priority, so equal-priority items at the same anchor had no real tie-break and just fell back to whatever order the HashMap felt like that frame. Every re-query on scroll could shuffle them, hence the jumping pipes.

Fix is just adding VirtualTextId (insertion order) as the final tie-break, so same position + same priority always resolves consistently and the rows stop reordering.

@sinelaw

sinelaw commented Jun 23, 2026

Copy link
Copy Markdown
Owner

Same here as in the other PR - I can't reproduce the scroll issue causing table borders to move.

Can we change this PR to only add the ConcealManager stuff so I can merge it separately?

@sinelaw

sinelaw commented Jun 23, 2026

Copy link
Copy Markdown
Owner

By the way which OS / Platform / Terminal app are you using?

@Agrejus

Agrejus commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Same here as in the other PR - I can't reproduce the scroll issue causing table borders to move.

Can we change this PR to only add the ConcealManager stuff so I can merge it separately?

I wonder if it is specific to my setup, I would think so

@Agrejus

Agrejus commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

By the way which OS / Platform / Terminal app are you using?

I am using:

PC: Mac Book Pro M1
OS: Tahoe 26.3
Platform: Apple Silicon arm64
Terminal App: Ghostty running tmux

Agrejus added a commit to Agrejus/fresh that referenced this pull request Jun 24, 2026
…primitive

Per @sinelaw's review on sinelaw#2399: pare this PR down to only the
ConcealManager namespaced range-clear so it can be merged on its own.
The deterministic VirtualTextId tiebreak in virtual_text.rs is removed
here and will be revisited separately (the underlying scroll/flicker
issue isn't reproducible on the maintainer's setup).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Agrejus Agrejus changed the title Namespaced conceal range-clear + deterministic virtual-line ordering Namespaced conceal range-clear (ConcealManager::remove_in_range_for_namespace) Jun 24, 2026
@Agrejus

Agrejus commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Same here as in the other PR - I can't reproduce the scroll issue causing table borders to move.

Can we change this PR to only add the ConcealManager stuff so I can merge it separately?

Yes, completed

@sinelaw

sinelaw commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Needs rebase

Agrejus and others added 2 commits June 24, 2026 18:11
…ne order

Two small, additive core primitives the markdown-compose work depends on,
split out of the larger sinelaw#2325 so they can be reviewed and merged on their own.

- `ConcealManager::remove_in_range_for_namespace()` — clear only one
  namespace's conceal ranges overlapping a byte range, leaving other plugins'
  conceals intact. Mirrors the existing `Overlay` equivalent (issue sinelaw#2146).
  Exposed to plugins as `editor.clearConcealsInRangeForNamespace(...)` via a
  new `PluginCommand::ClearConcealsInRangeForNamespace`. Lets a plugin rebuild
  its own conceals per line without stomping another plugin's.

- `VirtualTextManager` query methods now break sort ties on a monotonic
  `VirtualTextId` (insertion order) after (position, priority). Virtual texts
  live in a HashMap with arbitrary iteration order, so equal-(position,
  priority) entries at one anchor — e.g. several stacked continuation rows of
  one wrapped table row — previously came back shuffled between frames.

No graphics/kitty code here. Pure additions; existing behavior unchanged.

Tests: namespace isolation + range/version semantics for the conceal clear;
insertion-order determinism for same-anchor/same-priority virtual lines.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…primitive

Per @sinelaw's review on sinelaw#2399: pare this PR down to only the
ConcealManager namespaced range-clear so it can be merged on its own.
The deterministic VirtualTextId tiebreak in virtual_text.rs is removed
here and will be revisited separately (the underlying scroll/flicker
issue isn't reproducible on the maintainer's setup).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sinelaw sinelaw force-pushed the agrejus/core-decoration-primitives branch from 01c3253 to 104f5a0 Compare June 24, 2026 15:11
@sinelaw sinelaw merged commit 3432417 into sinelaw:master Jun 24, 2026
8 checks passed
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.

2 participants