Menu
▾
▴
#132 Incorrect tab width under GESHI_HEADER_PRE
open-accepted
5
2012-12-25
2009-04-29
No
When I specify a custom tab width — for example, $geshi->set_tab_width(4) — GeSHi creates the proper number of spaces when I also specify $geshi->set_header_type(GESHI_HEADER_DIV), but not when I allow it to default to PRE. I'd prefer to use <pre> tags and eliminate loads of from the code, but the default tab width of 8 doesn't work so well when displaying Objective-C code, which is already relatively verbose.
Seems the processing of Tabs is skipped with PRE headers.
There are two arguments for this, one for and one against this behaviour:
- The one against this is, as you mentioned, the ignored value for the tab width
- The one in favor of this is that pre means pre-formatted
I'll look into this, might take some time though.
The arguments each way are interesting, although I personally find skipping the tab spacing in pre tags to be inflexible and counter-intuitive. Yes, it's easier to leave the code intact, but I believe that even in pre tags, tab width should be user-configurable. I think of pre as a way to preserve the spacing of the markup without having to substitute non-breaking spaces, not as some unspoken rule that the markup must be identical to the original text source.
Many text editors allow you to set the representational width of tabs, and I prefer to use tabs because I can make them look 4 spaces wide, or any other width I like. Adjustable tab width is more important in some cases than others — for example, I frequently code in Objective-C, which tends to be a somewhat verbose language, and 8-space tabs make it even wider and can cause lots of scrolling/wrapping (depending on the CSS), which makes the code harder to read.
I definitely vote to have GeSHi respect the configured tab width regardless of whether it's in a div or pre block. Thanks!
As far as timeline, I didn't necessarily expect an immediate fix. I tend to think that placing demands and high expectations on authors of open-source software to be presumptuous. :-)
Sorry, that last comment was from me, I just wasn't logged in but it still allowed me to post.
Well, the fastest way to have a feature integrated into an OSS project in my experience is writing a small patch that exactly does this ;-)
But well, regarding the text editors: Yes, they all support it and most even do support converting from one way to the other.
But one point that really hits with this change is performance, as GeSHi usually tries to avoid work that doesn't need to be done, like don't match numbers if there aren't any and such things. Reworking the tabs requires GeSHi to read through each line and count all the chars there to see where to insert how many spaces. This can be an awful lot of time.
If the change is done (probably for 1.0.8.5) then this won't be the default behaviour for set_tab_width, but instead an extra parameter to enforce it.
I've been an active patcher on projects from time to time, and I'd love to take a look at this, but I haven't had time to devote to it. (On the other hand, I *did* just contribute a language file for BibTex — still owe you some test files on that one...)
In the end it's your call, and I'll try not to get religious about this, but it seems to me that having *another* config option to say "yes, really" is a bad idea. Since the default tab width is 8 spaces (both in GeSHi and in most browsers), spending time to parse tabs and replace with spaces won't even be an issue unless the user has already specified a tab width other than 8. It's true that pre is the default, and as such, combining tab width with the other container behaviors means more confusing setup.
If I understand correctly, do *none* of the pre modes do tab replacement, only div? The inconsistency is a potential point of confusion. Also, it sounds like you're describing more work than I'd expect it to be — I'd think tab width would only have to recognize *tabs* and replace them, not count all the chars on a line, but I could be mistaken.
Bottom line, if the user configures GeSHi with a given tab width, I think they'd expect to see it, and understand if it's a few milliseconds slower. As evidenced by this bug report, I was confused by tab width *not* working inside pre tags. Despite the good intentions of avoding unnecessary work, to the user it seems more like a bug than a feature...
For the patch you might simply add a second parameter to set_tab_width ($enforce=false) and a new var $tab_width_enforce = false; near the one for the tab width.
The reast simply means tweaking the conditions in which the rework is done + adding a replacement of --> space.
For the counting:
Consider ab{tab}cd with tab width 4:
You can't simply replace the tab with 4 spaces, but have to count the chars before the tab and snap to the next grid. That's what takes the time. Simply replacing is not much work.