Align tabs to the next tab stop instead of a fixed advance - #611
Align tabs to the next tab stop instead of a fixed advance#611ollieb-unity wants to merge 1 commit into
Conversation
TabTextElement handed a literal "\t" to the formatter, but the text shaper gives every tab glyph the same advance (TextShaperOptions.IncrementalTabWidth) with no knowledge of the column the tab starts at. A tab therefore always advanced IndentationSize spaces rather than reaching the next multiple of it, so text after a tab only lined up when it happened to start on a tab stop. The generator now measures the column where the tab starts, expanding any earlier tabs on the same line, and emits the spaces needed to reach the next tab stop. The element's visual length varies with that count, and CreateTextRun handles a run request landing part way into the element, which happens when the line wraps. The zero width TabGlyphRun is unchanged, so the fix for AvaloniaUI#206 still holds. TextView now always attaches the generator, because it owns tab layout and could otherwise be dropped when all three whitespace glyph options are off.
There was a problem hiding this comment.
Pull request overview
Updates tab rendering so tabs advance to the next indentation-aligned tab stop.
Changes:
- Expands tabs into calculated spaces.
- Keeps tab layout active when whitespace markers are hidden.
- Adds rendering tests for common tab positions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
SingleCharacterElementGenerator.cs |
Calculates tab width and emits spaces. |
TextView.cs |
Always enables the tab-layout generator. |
TextViewTests.cs |
Tests tab-stop alignment. |
Suppressed comments (3)
src/AvaloniaEdit/Rendering/SingleCharacterElementGenerator.cs:153
- This computes a document-text column rather than the tab's visual column. A preceding custom/folding element can replace many document characters with a different
VisualLength, and a fold spanning lines can make this reset at a laterDocumentLineeven though rendering continues on the same visual line. Tabs after such an element therefore receive the wrong number of spaces. Calculate from the visual elements already emitted (or expose the current construction visual column) instead of rescanning raw document text.
var line = context.Document.GetLineByOffset(offset);
var textBeforeTab = context.GetText(line.Offset, offset - line.Offset);
var column = 0;
for (var i = 0; i < textBeforeTab.Count; i++)
{
column += textBeforeTab.Text[textBeforeTab.Offset + i] == '\t'
? indentationSize - column % indentationSize
: 1;
src/AvaloniaEdit/Rendering/SingleCharacterElementGenerator.cs:149
- Each tab requests and rescans the entire prefix of its line.
VisualLineTextSource.GetTextcannot reuse its cached string for each successively longer prefix, so a tab-dense line causes quadratic scanning and allocations on every visual-line rebuild (up to the 3000-character generator limit). Track the running column while offsets are constructed, or cache prefix columns in a single pass.
var line = context.Document.GetLineByOffset(offset);
var textBeforeTab = context.GetText(line.Offset, offset - line.Offset);
var column = 0;
for (var i = 0; i < textBeforeTab.Count; i++)
src/AvaloniaEdit/Rendering/SingleCharacterElementGenerator.cs:205
- Literal spaces do not preserve AvaloniaEdit's tab-width contract for proportional fonts.
TextView.WideSpaceWidthintentionally uses the width ofxbecause actual spaces can be much narrower, and the previous formatter tab size wasIndentationSize * WideSpaceWidth; theseTextCharactersinstead advance by the font's ordinary space width, so rendered text will still miss the tab-stop/column-ruler grid. Use a run whose advance is based onWideSpaceWidth(and add a proportional-font assertion) rather than relying on space glyph advances.
var remainingSpaceCount = VisualLength - relativeOffset;
return new TextCharacters(
_spaces.AsMemory(_spaces.Length - remainingSpaceCount), TextRunProperties);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (ShowTabs) | ||
| return startOffset + i; | ||
| break; | ||
| return startOffset + i; |
|
IncrementalTabWidth is a fixed width for the tab character. This does not align anything to a specific column layout. This is working as intended. For column layouts, there needs to be an array of tab stops so one can define a width individually. |
|
Thanks @Gillibald yeah I think the fix belong to this project and we should assume that AvaloniaEdit uses monospaces font (same as VSCode), so I think this is the right approach. @ollieb-unity thanks for the PR. I reviewed it, and I think we can make a couple of improvements:
I will handle those changes. I think I have a clear idea. |
|
I've opened #612 as a rewrite with the changes I mentioned above. |
Problem
Text after a tab does not line up on a tab stop.
TabTextElementemitsTextCharacters("\t")and leaves the width to the formatter, but the text shaper gives every\tglyph a constant advance (TextShaperOptions.IncrementalTabWidth, seesrc/Skia/Avalonia.Skia/TextShaperImpl.csin Avalonia) with no knowledge of the column the tab starts at. A tab therefore always advancesIndentationSizespaces instead of reaching the next multiple of it, so text after a tab only lines up when it happens to start on a tab stop already.This shows up in any file that uses tabs to align trailing comments or declarations. Taking eight declarations whose tab counts make the comments align at tab size 4, here is the column each comment lands on:
The comments drift one column per line and snap back every fourth line. Leading indentation looks correct either way, which is why this is easy to miss: a tab at column 0, 4, 8 lands on a stop under both behaviours.
Change
SingleCharacterElementGeneratormeasures the column where the tab starts, expanding any earlier tabs on the same line, and emits the spaces needed to reach the next multiple ofIndentationSize, rather than handing"\t"to the formatter.TabTextElement.VisualLengthnow varies with that count, andCreateTextRunhandles a run request that lands part way into the element. That happens when the line wraps, and the old two column element could not express it.TabGlyphRunis untouched, so the fix from Fix tab glyph extra width #207 / Tab sign change the position of line #206 (tab glyph extra width) still holds. The arrow is still drawn as an overlay at the tab's start and contributes no width.ShowTabsis set, since the width was wrong in both cases. ConsequentlyTextViewnow always attaches the generator: it owns tab layout and could otherwise be dropped whenShowSpaces,ShowTabsandShowBoxForControlCharactersare all off.Test
TextViewTests.Tab_Should_Reach_The_Next_Tab_Stopasserts the rendered runs for three cases atIndentationSize = 4: a tab from column 1 and one from column 3 both reach column 4, and a tab already sitting on a stop reaches the next one. It asserts run text rather than pixel positions, so it does not depend on font metrics.Without the source change the test fails with
Expected: "a b" But was: "a\tb". The full suite passes with it (516 tests).Two things worth your opinion
IncrementalTabWidthas a tab stop instead of a constant advance, the originalTextCharacters("\t")would be correct and this change would be unnecessary. I fixed it in AvaloniaEdit because AvaloniaEdit already owns aTabTextElementand decides how tabs render, but I am happy to move it upstream into Avalonia if you would rather have it there.TextViewchange alters default behaviour for every consumer, since the generator becomes always present. It is needed for the fix to apply when the whitespace glyphs are hidden. Happy to split it into its own commit or drop it if you would prefer to keep tabs broken in that configuration for now.