Skip to content

Conversation

@benoitkugler
Copy link
Contributor

@benoitkugler benoitkugler commented Dec 12, 2025

This PR is a first step towards support for tabs (for #174).

The only API added is the Line.AlignTabs method which aligns all tabs of a given line.

This seems enough if you only wrap text one line at a time (typically using shaping.NewSliceIterator and Wrapper.WrapNextLine), which is my use case.

Supporting tabs when wrapping a whole paragraph seems harder, since we have to align tabs again at the start of every line. The non exported Output.applyTabs should probably be used every time we "consume" a new shaped run.

@andydotxyz
Copy link
Contributor

Supporting tabs when wrapping a whole paragraph seems harder, since we have to align tabs again at the start of every line.

An interesting point. Do we? In the work I have done elsewhere the tab alignment is referring to the first line (where the actual tab character was) and the others have a 0 offset.

However it is a little dependent on use-case and may not always follow. However a more complete solution is how many taps the first and "all others" indent as two options I think?

There is the complication also of where the run does not start at the beginning of the paragraph so the initial indent is less due to previous content rather than because of the tab characters...

@benoitkugler
Copy link
Contributor Author

An interesting point. Do we? In the work I have done elsewhere the tab alignment is referring to the first line (where the actual tab character was) and the others have a 0 offset.

Even if we keep the tab alignement defined by the first line, I think we would have to adjust tabs (that is mutate tab glyphs advances) since the wrapping might have change the relative position between runs (in the next line) and tabs grid. For instance if we cut a line in the middle of a "tab column".

@benoitkugler
Copy link
Contributor Author

There is the complication also of where the run does not start at the beginning of the paragraph so the initial indent is less due to previous content rather than because of the tab characters...

Good point. So perhaps we should add a parameter indicating the start of the first column tab, which typically defaults to 0 ?

@andydotxyz
Copy link
Contributor

An interesting point. Do we? In the work I have done elsewhere the tab alignment is referring to the first line (where the actual tab character was) and the others have a 0 offset.

Even if we keep the tab alignement defined by the first line, I think we would have to adjust tabs (that is mutate tab glyphs advances) since the wrapping might have change the relative position between runs (in the next line) and tabs grid. For instance if we cut a line in the middle of a "tab column".

I guess the challenge of the mutating advance approach is that the advance is only correct in the context of the initial offset - i.e. depending how much the first line is offset (or the paragraph) will change how the tab advance will be applied...

@andydotxyz
Copy link
Contributor

There is the complication also of where the run does not start at the beginning of the paragraph so the initial indent is less due to previous content rather than because of the tab characters...

Good point. So perhaps we should add a parameter indicating the start of the first column tab, which typically defaults to 0 ?

Yes that sounds good. Perhaps the first line is a parameter and the other lines is a second, which might cover the previous point too?

whereswaldon and others added 4 commits December 23, 2025 13:54
* [shaping] expose the space we trim when wrapping with TrimWhiteSpace enabled

* minor typos

* Replace setup-go-faster action with setup-go

* Replace setup-go-faster action with setup-go

* [shaping] expose the space we trim when wrapping with TrimWhiteSpace enabled

* minor typos

* remove debug fmt.Println
@benoitkugler
Copy link
Contributor Author

So, we now have func (l Line) AlignTabs(text []rune, columnWidth, lineOffset fixed.Int26_6).
This method is intended for simple one by one line wrapping; supporting whole paragraph layout is a future work.

Copy link
Contributor

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Looks good to me, and the tests appear correct.
I have not had a chance to test it in any real usage, but hope to in a while to see if we can move off the Fyne-specific wrapping.

Copy link
Member

@whereswaldon whereswaldon left a comment

Choose a reason for hiding this comment

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

I'm not sure that I understand the expected use of the API. We define AlignTabs on Line, which is the output of line-wrapping. However, anything that modifies the advance of glyphs/runs needs to happen prior to line wrapping in order to ensure correct wrapped results. Could you provide an example of how this is meant to work in tandem with line wrapping?

@benoitkugler
Copy link
Contributor Author

I'm not sure that I understand the expected use of the API. We define AlignTabs on Line, which is the output of line-wrapping. However, anything that modifies the advance of glyphs/runs needs to happen prior to line wrapping in order to ensure correct wrapped results. Could you provide an example of how this is meant to work in tandem with line wrapping?

I understand your concern : the AlignTabs method should be called before line wrapping, actually on a slice of runs. I've used Line just as such a slice, not as the output of line wrapping, but it may be confusing. Perhaps it should be a function
AlignTabs(line []Output)
instead ?

@whereswaldon
Copy link
Member

I'm not sure that I understand the expected use of the API. We define AlignTabs on Line, which is the output of line-wrapping. However, anything that modifies the advance of glyphs/runs needs to happen prior to line wrapping in order to ensure correct wrapped results. Could you provide an example of how this is meant to work in tandem with line wrapping?

I understand your concern : the AlignTabs method should be called before line wrapping, actually on a slice of runs. I've used Line just as such a slice, not as the output of line wrapping, but it may be confusing. Perhaps it should be a function AlignTabs(line []Output) instead ?

But aren't the results of tab alignment wrapping-dependent? If there's a tab in the middle of line 2, the advance of that tab is dependent on the advance of the prior text on line two, as it needs to push the next glyph to be tabstop-aligned. I don't think we can a-priori determine how much text will be before a tab in a given line, so we can't compute the advances before wrapping. Am I missing something?

Based on my current understanding, tab expansion needs to happen during wrapping as part of processing a run to see if it can fit on the current line. I would be happy to be corrected though, as I'm not eager to mess with that part of the logic.

@benoitkugler
Copy link
Contributor Author

I'm not sure that I understand the expected use of the API. We define AlignTabs on Line, which is the output of line-wrapping. However, anything that modifies the advance of glyphs/runs needs to happen prior to line wrapping in order to ensure correct wrapped results. Could you provide an example of how this is meant to work in tandem with line wrapping?

I understand your concern : the AlignTabs method should be called before line wrapping, actually on a slice of runs. I've used Line just as such a slice, not as the output of line wrapping, but it may be confusing. Perhaps it should be a function AlignTabs(line []Output) instead ?

But aren't the results of tab alignment wrapping-dependent? If there's a tab in the middle of line 2, the advance of that tab is dependent on the advance of the prior text on line two, as it needs to push the next glyph to be tabstop-aligned. I don't think we can a-priori determine how much text will be before a tab in a given line, so we can't compute the advances before wrapping. Am I missing something?

Based on my current understanding, tab expansion needs to happen during wrapping as part of processing a run to see if it can fit on the current line. I would be happy to be corrected though, as I'm not eager to mess with that part of the logic.

You are completely right, that is why I have not tackled the general case on this PR.
But in my use case, I only wrap one line at a time. In this case, line wrapping does not break tabs (at least in LTR direction, which is already nice enough for me).

@whereswaldon
Copy link
Member

whereswaldon commented Jan 29, 2026

You are completely right, that is why I have not tackled the general case on this PR. But in my use case, I only wrap one line at a time. In this case, line wrapping does not break tabs (at least in LTR direction, which is already nice enough for me).

I'm sorry, I clearly didn't read the PR description thoroughly today. You already covered this. 🤦‍♂️

However, since expanding tabs changes the width of the line, I fail to see how it can be safe even for the line-by-line use-case. If you only call WrapNextLine(), you will still sometimes get a line that is packed full, and within which a tab expansion should change the wrapping decision. I believe that is possible even if the text is all the same direction. Is there some mitigation that I'm missing to handle that case?

@benoitkugler
Copy link
Contributor Author

benoitkugler commented Jan 29, 2026

You are completely right, that is why I have not tackled the general case on this PR. But in my use case, I only wrap one line at a time. In this case, line wrapping does not break tabs (at least in LTR direction, which is already nice enough for me).

I'm sorry, I clearly didn't read the PR description thoroughly today. You already covered this. 🤦‍♂️

However, since expanding tabs changes the width of the line, I fail to see how it can be safe even for the line-by-line use-case. If you only call WrapNextLine(), you will still sometimes get a line that is packed full, and within which a tab expansion should change the wrapping decision. I believe that is possible even if the text is all the same direction. Is there some mitigation that I'm missing to handle that case?

Hum. If you apply tabs on the whole slice of runs, that is you pretend you have one "infinite" line, and then you just call WrapNextLine to cut the line, I'm failing to see what could be wrong. Perhaps I'm simplifying the situation too much ? Have you a counter example in mind ?

@whereswaldon
Copy link
Member

You are completely right, that is why I have not tackled the general case on this PR. But in my use case, I only wrap one line at a time. In this case, line wrapping does not break tabs (at least in LTR direction, which is already nice enough for me).

I'm sorry, I clearly didn't read the PR description thoroughly today. You already covered this. 🤦‍♂️
However, since expanding tabs changes the width of the line, I fail to see how it can be safe even for the line-by-line use-case. If you only call WrapNextLine(), you will still sometimes get a line that is packed full, and within which a tab expansion should change the wrapping decision. I believe that is possible even if the text is all the same direction. Is there some mitigation that I'm missing to handle that case?

Hum. If you apply tabs on the whole slice of runs, that is you pretend you have one "infinite" line, and then you just call to WrapNextLine to cut the line, I'm failing to see what could be wrong. Perhaps I'm simplifying the situation too much ? Have you a counter example in mind ?

I'm having trouble thinking clearly about your approach. Can you share code or pseudocode of exactly how you're using WrapNextLine() and AlignTabs() in tandem?

@benoitkugler
Copy link
Contributor Author

benoitkugler commented Jan 29, 2026

The following assumes we have shaped the text to get outputs, a slice of Output

// segment and shape the input text
outputs := []Output{ ... } // elided

// align tabs
tabWidth := fixed.I(style.TabSize.Width)
outputs.AlignTabs(text, tabWidth)

// now we can wrap the runs
config := shaping.WrapConfig{ ... }  // configuration elided
lineWrapper.Prepare(config, text, shaping.NewSliceIterator(outputs))
wLine, fitsOnFirstLine := lineWrapper.WrapNextLine(mw)
line := wLine.Line

// use line

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.

4 participants