Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elastic tabstop support #730

Closed
nrdxp opened this issue Sep 8, 2021 · 9 comments
Closed

Elastic tabstop support #730

nrdxp opened this issue Sep 8, 2021 · 9 comments
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements R-wontfix Not planned: Won't fix

Comments

@nrdxp
Copy link
Contributor

nrdxp commented Sep 8, 2021

Describe your feature request

The header to the article first introducing elastic tabstops reads "The status quo sucks", and I can't help but agree. Much more than a simple visual improvement, I feel the concept helps to introduce a semantically meaningful purpose for distinguishing proper usage for tabs vs spaces.

Implementing this may not even be too difficult since the ripgrep author seems to have already written a fairly decent library for this. The idea was first brought to my attention by @ashkitten on the matrix chat, but hopefully this issue will help it become a reality sooner rather than later.

@nrdxp nrdxp added the C-enhancement Category: Improvements label Sep 8, 2021
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Sep 9, 2021
@kirawi kirawi added E-good-first-issue Call for participation: Issues suitable for new contributors E-medium Call for participation: Experience needed to fix: Medium / intermediate and removed E-good-first-issue Call for participation: Issues suitable for new contributors labels Nov 7, 2021
@pascalkuthe
Copy link
Member

pascalkuthe commented Dec 15, 2022

#5008 is related to this and will probably make implementing this feature a lot easier once it lands as it deals with some of the same problems (but more generically)

@Bruce-Hopkins
Copy link
Contributor

@pascalkuthe how exactly should the editor handle this? Will the code itself be formatted to elastic tabstops? Or will there be virtual text?

@pascalkuthe
Copy link
Member

pascalkuthe commented Feb 24, 2023

@pascalkuthe how exactly should the editor handle this? Will the code itself be formatted to elastic tabstops? Or will there be virtual text?

Not really #5420 (replaced #5008 and was merged into master) centralizes text positoning to a single function so instead of a bunch of spread-out places dealing with tabs. Variable tab-width can now be implemented in a single place (the document formatter) and should automatically work everywhere else (the indentation code is an exception but that should be rather easy to adjust).

That being said I looked into the details a bit more and this feature is actually very tricky to implement. It requires significant backtracking in the rendering/positoning code. While the linked articel claims that this should not be a concern for modern hardware I believe that is not accurate. Helix rendering/positoning code is carefully design so that its context free meaning that the editor only needs to backtrack to the start of the current line (even less in the future) to determine the horizontal on-screen position of a grapheme and to the first visible character on the screen to also determine the vertical position (relative vertical offsets only require backtracking to the current line).

However with elastic tabstop the tabwidth is actually dependent on the entire document, violating this concept of locality. Therefore rendering would require always backtracking to the start of the document (you could transverse the text backwards too but then you risk even more unnecessary work). Transversing the document contents is already a performance bottelneck during rendering (especially for files with very long lines) and elastic tabstops would make that problem orders of magnitude worse. The only way I see us achieving acceptable performance is if we cache the elastic tabstop computation by essentially storing an alignment grid on the document. We have intentionally avoided adding such caches to helix because they are insanely hard to keep consistent and add a huge maintainance burden. Even then elastic tabstops would need to be computed on every keypress which can still cause significanty overhead.

Finally elastic tabstops can fundamentally not work with softwrap because softwrap needs to know how long a grapheme is to decide at which horizontal/vertical position to place it. However with elastic tabstop the width of tabs depends on it's position in the document. This would turn elastic tabstop computation to a global fixpoint analysis if we want to account for softwrap (which unacceptable to run on every keypress for performance reasons even if converges).

I believe while the idea of elastic tabstops sounds neat at first violating the locality of text positioning so fundamentally is not something we should do. I think a much better solution would be to make the tab key insert indent units (be it spaces or tabs) until the text is aligned according to he rules of elastic tabstop. This not only has the advantage of keeping all this complexity out of the editor core but also keeps the files edited this way readable outside of helix.

One final note on tabwriter: This crate was never intended for use in a textedtor. Instead its intended for transforming text when reading from/to CSV files (so it converts String -> String essentailly). This does not work at all for an editor.

@eugenesvk
Copy link

However with elastic tabstop the tabwidth is actually dependent on the entire document, violating this concept of locality.

You could have a more performant version that only depends on the ±X pages instead of the entire document

Even then elastic tabstops would need to be computed on every keypress which can still cause significanty overhead.

There is also a simple workaround for this - instead of computing on every keypress you compute after Xsec passed since the last edit. This works nicely in the Sublime Text's pseudo-elastic tabstops implementation via a plugin that uses spaces for alignment, and it's expensive to insert/delete a lot of spaces in a lot of lines all the time

Finally elastic tabstops can fundamentally not work with softwrap because softwrap needs to know how long a grapheme is to decide at which horizontal/vertical position to place it. However with elastic tabstop the width of tabs depends on it's position in the document.

Can this be addressed by "decreasing" the precision of softwraps, e.g., by allowing some lines where a tab is accidentally 16 units instead of 2 to just partially flow offscreen while a less dramatic jump would work more or less fine?

I think a much better solution would be to make the tab key insert indent units (be it spaces or tabs) until the text is aligned according to he rules of elastic tabstop.

Would that not be even worse from the performance standpoint due to the need to edit so many lines at once (to ensure alignment of all the adjacent lines)? Sublime has a plugin that does exactly this, and it's unusable on longer chunks of vertically aligned texts without adding delays so that it doesn't do a lot of edits on every keystroke (granted, there is no alternative proper implementation to compare the speed to)

@pascalkuthe
Copy link
Member

However with elastic tabstop the tabwidth is actually dependent on the entire document, violating this concept of locality.

You could have a more performant version that only depends on the ±X pages instead of the entire document

This is not that easy. How do you define pages? By document lines? That doesn't work for heavily softwrapped files (which are also usually the most cirtical from a performance standpoint). If you want to compute the actual height you need to transverse the lines which again is very expensive from a performance standpoint.

Even then elastic tabstops would need to be computed on every keypress which can still cause significanty overhead.

There is also a simple workaround for this - instead of computing on every keypress you compute after Xsec passed since the last edit. This works nicely in the Sublime Text's pseudo-elastic tabstops implementation via a plugin that uses spaces for alignment, and it's expensive to insert/delete a lot of spaces in a lot of lines all the time

This would not be great from a usability standpoint as it would randomly shift the text on idle timeout. The much more important point here is:

We have intentionally avoided adding such caches to helix because they are insanely hard to keep consistent and add a huge maintainance burden.

I hand-waved a lot of VERY hard details here. How do you actually cache the text width? A HashMap<usize, usize>? That has horrible performance compared to the Rope we are using. It would be a huge pain to update and maintain.

Finally elastic tabstops can fundamentally not work with softwrap because softwrap needs to know how long a grapheme is to decide at which horizontal/vertical position to place it. However with elastic tabstop the width of tabs depends on it's position in the document.

Can this be addressed by "decreasing" the precision of softwraps, e.g., by allowing some lines where a tab is accidentally 16 units instead of 2 to just partially flow offscreen while a less dramatic jump would work more or less fine?

Theoretically this would solve the problem but this solution is entirely unacceptable to me. It would make softwrap completly useless when elastic tabstops are enabled. The problem doesn't just occur sometimes for large tabs it would be a problem for every single tab. Most lines would endup off screen by one or two chars. I also mostly entertained the caching approach as a hypothetical. The effort required would be huge and require drastic changes to the codebase.

I think a much better solution would be to make the tab key insert indent units (be it spaces or tabs) until the text is aligned according to he rules of elastic tabstop.

Would that not be even worse from the performance standpoint due to the need to edit so many lines at once (to ensure alignment of all the adjacent lines)? Sublime has a plugin that does exactly this, and it's unusable on longer chunks of vertically aligned texts without adding delays so that it doesn't do a lot of edits on every keystroke (granted, there is no alternative proper implementation to compare the speed to)

I only meant that tab would insert tabs/spaces to align with line above not that other lines alignment would be changed to match the previous line. Altough I gues that wouldn't really give you most of the benefits. We could align other lines too but that might indeed be a performance problem for large files (altough the Rope in helix is much more efficent than whatever sublime seems to be doing. 10k edits shouldn't not be a problem). In general the important part was also:

but also keeps the files edited this way readable outside of helix.

Helix will not the change the world to use elastic tabstops. I dont think its worth the effort to introduce such a complicated feature with huge performance caveats if any file edited this way is not even readable by people using.... Any other editor. It's already a huge problem that varios tools can not really agree on the width of unicode text. Lets not make that even worse by also changing the width of a tab (and making it context dependent too)

@eugenesvk
Copy link

How do you define pages? By document lines?

Yes

That doesn't work for heavily softwrapped files (which are also usually the most cirtical from a performance standpoint). If you want to compute the actual height you need to transverse the lines which again is very expensive from a performance standpoint.

Why doesn't it work? You don't care about "actual height", most of what you care about is that while moving up/down in the vicinity of your editing the alignment stays, so it doesn't matter that your ±80 lines is not ±1 page, but 4 pages because one line is long enough to occupy the whole page soft-wrapped, no need to calculate the precise number of lines for it, 80 would still be fine
Then there are use cases like TSVs where you might need to account for the whole document, but there it's most likely that you don't use soft-wraps as they'd break your tables

This would not be great from a usability standpoint as it would randomly shift the text on idle timeout.

It's not random, it's constant time right after you've stoped typing. And it's actually better for "usability" vs. constant shifting. I've tried both ways in ST and like the one with a timeout better (even outside the performance factor). While you're continuously editing the line, those jumps are more of a distraction

We have intentionally avoided adding such caches to helix because they are insanely hard to keep consistent and add a huge maintainance burden.

I hand-waved a lot of VERY hard details here. How do you actually cache the text width? A HashMap<usize, usize>? That has horrible performance compared to the Rope we are using. It would be a huge pain to update and maintain.

Cache was your response to having to traverse the entire document, but this isn't necessary, so maybe cache isn't needed?
Otherwise, not sure what the best data structure for that is.

Theoretically this would solve the problem but this solution is entirely unacceptable to me. It would make softwrap completly useless when elastic tabstops are enabled. The problem doesn't just occur sometimes for large tabs it would be a problem for every single tab. Most lines would endup off screen by one or two chars.

Doesn't seem that catastrophic, you'd just set the width smaller by a few chars, so most lines would not be off screen, little price to pay for the beauty of automatic tables

I only meant that tab would insert tabs/spaces to align with line above not that other lines alignment would be changed to match the previous line.
Altough I gues that wouldn't really give you most of the benefits.

Yes, this is completely useless, the whole point of elastic tabstops is that it keeps alignment of the whole text block when you edit any line in the block

In general the important part was also:

but also keeps the files edited this way readable outside of helix.

Helix will not the change the world to use elastic tabstops.

I don't get the importance of this, the world can remain ugly alignment-wise, why does it conceptually prevent you from wanting your text/editor to look nice?

Also, it is readable, it's just readable in the same way as almost all the code written today (with all the autoformatters etc.) is — as an unaligned zig-zaggy mess like

a = 1
bbbbb = 1 
ccc = 2

... which, given that his style is so widespread, is obviously readable

Any other editor. It's already a huge problem that varios tools can not really agree on the width of unicode text.

Sure, but you wouldn't ban unicode because of this!

Lets not make that even worse by also changing the width of a tab (and making it context dependent too)

The width is already different per user as it's configurable in many editors. The context dependency is new indeed, though since the bad part is lack of alignment, adding some alignment even if it's not recognized by others is an improvement, not making it worse

with huge performance caveats

But they wouldn't be huge with a few tweaks? I mean, even the poor Sublime's implementation is ok most of the time since most of the time the alignment area is a dozen lines, but then I've also tried the much more performant (and also more correct implementation that doesn't insert any spaces) Notepad++'s version, and that one performs poorly on hundreds of lines (though it doesn't have the delay, so it does the expensive stuff on every edit, which is avoidable; it also has an option to disable taking the whole document into account)

@dead10ck
Copy link
Member

dead10ck commented Feb 26, 2023

@eugenesvk

I don't get the importance of this, the world can remain ugly alignment-wise, why does it conceptually prevent you from wanting your text/editor to look nice?

A key piece you're not getting is that code is fundamentally collaborative. If no one but you can see it rendered nicely, then the whole thing is pointless.

According to the web page for Elastic Tabstops, they have been a thing since 2006. If after 17 years they are still not adopted by most software, this should be a big clue that it's not a good, or at least practical, idea.

Pascal just did a bunch of fundamental work on helix's text rendering system; you can trust that he knows what he's talking about when he says it is extremely complicated and not worth the effort, and not pick apart every sentence he writes, especially when you are not the one who will be doing the work to make it happen.

@eugenesvk
Copy link

If no one but you can see it rendered nicely, then the whole thing is pointless.

That simply makes no sense, it's like saying I shouldn't use a nice font (that also has a different width/height, so this also affects the alignment slightly) because no one can see it rendered nicely besides me in my editor.
The point is that people who value alignment will enjoy it. People not caring about it will continue to mostly see the same thing they currently see.

If after 17 years they are still not adopted by most software, this should be a big clue that it's not a good, or at least practical, idea.

Yeah, right, like the fact that QWERTY is everywhere should tell us that alternative layouts aren't a good idea. Or the fact that "most software" hasn't adopted the modal paradigm for text editing clues us into how it's not good

Pascal just did a bunch of fundamental work on helix's text rendering system; you can trust that he knows what he's talking about when he says it is extremely complicated and not worth the effort, and not pick apart every sentence he writes, especially when you are not the one who will be doing the work to make it happen

Or I can leave the blind trust part to you and correct the mistakes I see in his reasoning: would be a shame if this feature didn't get implemented because of them (even more so because I'm not the one who will be doing the work)

@archseer
Copy link
Member

and correct the mistakes I see in his reasoning: would be a shame if this feature didn't get implemented because of them

Maybe rather than hand-waving away the difficulties and adding more work to Pascal's plate you could stop arguing with us, implement the feature yourself and open a PR?

@kirawi kirawi added E-hard Call for participation: Experience needed to fix: Hard / a lot R-wontfix Not planned: Won't fix and removed E-medium Call for participation: Experience needed to fix: Medium / intermediate E-hard Call for participation: Experience needed to fix: Hard / a lot labels Feb 20, 2024
@helix-editor helix-editor locked and limited conversation to collaborators Apr 7, 2024
@pascalkuthe pascalkuthe converted this issue into discussion #10224 Apr 7, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements R-wontfix Not planned: Won't fix
Projects
None yet
Development

No branches or pull requests

7 participants