Menu
▾
▴
[Tcl-bugs] [ tktoolkit-Bugs-852949 ] Infinite loop in tabstop calculations
|
From: SourceForge.net <no...@so...> - 2003-12-04 17:23:52
|
Bugs item #852949, was opened at 2003-12-02 15:22 Message generated for change (Comment added) made by jenglish You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=112997&aid=852949&group_id=12997 Category: 18. [text] Group: develop: 8.5a0 >Status: Open Resolution: Fixed Priority: 8 Submitted By: Joe English (jenglish) Assigned to: Jeffrey Hobbs (hobbs) Summary: Infinite loop in tabstop calculations Initial Comment: To replicate the problem: pack [text .t -tabs "100 50"] .t insert end "a\tb\t" This locks up Tcl. The root of the problem is that the last tab is smaller than the second-to-last tab, which makes tabArrayPtr->increment negative and the extrapolation loop in SizeOfTab() never terminates. ---------------------------------------------------------------------- >Comment By: Joe English (jenglish) Date: 2003-12-04 09:23 Message: Logged In: YES user_id=68433 > Fixed documentation and added tests, and fixed code to > throw an error rather than loop infinitely. tkdiff will need > to be fixed, now. While I agree that this is probably the Right Thing, it's also (as dkf points out) an incompatible change and ought not to be done in a minor release. This will break existing programs. Reopening. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2003-12-04 08:01 Message: Logged In: YES user_id=79902 Digging with CVS confirms that tabs have always (minimally since 8.0, though the changes file doesn't indicate any alteration in tab behaviour since 4.0 when they were introduced as controllable entitites) been done as offsets from the left edge of the widget. However, there has also always been code to make a tab minimally advance by the width of a space character. If we don't do that, we had better make sure (minimally) that we mark things as POTENTIAL INCOMPATABILITY... ---------------------------------------------------------------------- Comment By: Vince Darley (vincentdarley) Date: 2003-12-04 06:35 Message: Logged In: YES user_id=32170 p.s. of course tkdiff should use: $w(BottomText) configure -tabs [list [expr {$tabstops +($cwidth * 2)}] [expr {2*$tabstops +($cwidth * 2)}]] to function according to dkf's description. ---------------------------------------------------------------------- Comment By: Vince Darley (vincentdarley) Date: 2003-12-04 04:29 Message: Logged In: YES user_id=32170 Fixed documentation and added tests, and fixed code to throw an error rather than loop infinitely. tkdiff will need to be fixed, now. ---------------------------------------------------------------------- Comment By: Vince Darley (vincentdarley) Date: 2003-12-04 02:15 Message: Logged In: YES user_id=32170 Tab stops have always been relative to the widget edge, so tkdiff's code is simply wrong. I'll make non monotonic tabs throw an error and update the man page to clarify both (i) everything's relative to the widget and (ii) non increasing (or negative) tabs are an error. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2003-12-04 01:32 Message: Logged In: YES user_id=79902 It was never entirely clear to me whether the tab spacings relative to one another, were from the left margin, or from the left edge of the widget. However, the tkdiff code seems to be assuming what I always guessed - that tab-stops were all relative to the previous tab-stop. If that is indeed the way it worked, then that is the way it must work through 8.5 at least. The aim of the code is to simulate 8-char tab-stops (well, the 8 is configurable by altering tmpopts(tabstops) in this context) but offset the whole thing two characters further in so that there is an extra "column" (if you see what I mean) at the left edge that contains the "diff" markings (which are two characters wide.) Making all this code return an error is wrong since it will break much existing code in a minor version. Furthermore, tkdiff is a relatively high-profile app, so keeping it working is important. ---------------------------------------------------------------------- Comment By: Vince Darley (vincentdarley) Date: 2003-12-04 00:29 Message: Logged In: YES user_id=32170 Ok, I took a look at tkdiff and this is the offending code: # tabstops require a little extra work. We need to figure out # the width of an "m" in the widget's font, then multiply that # by the tab stop width. For the bottom text widget the first tabstop # is adjusted by two to take into consideration the fact that we # add two bytes to each line (ie: "< " or "> "). set cwidth [font measure [$w(LeftText) cget -font] "m"] set tabstops [expr {$cwidth * $tmpopts(tabstops)}] ... $w(BottomText) configure -tabs [list [expr {$tabstops +($cwidth * 2)}] $tabstops] To me, the above is clearly a bug. I have no idea what that that last line is supposed to achieve, but I can't see how it'll achieve any sensible goal in Tk 8.3/8.4 at all. So, from that perspective we would be better off throwing an error here so that this code gets fixed. ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2003-12-03 09:51 Message: Logged In: YES user_id=68433 > Joe: how did you encounter this problem? 'tkdiff' (v3.09) triggers the bug. I haven't figured out why or how tkdiff specifies nonmonotonically-increasing tab stops, but it does. (That's why I'd prefer that the text widget not raise an error for this condition; it would make (currently nonfatal) bugs in existing applications hard errors.) ---------------------------------------------------------------------- Comment By: Vince Darley (vincentdarley) Date: 2003-12-03 02:53 Message: Logged In: YES user_id=32170 Jeff, can I get an opinion on this? It it ok for us simply to throw an error here. It's clearly user error (and the documentation makes no mention of the implicit constraint that tabs must be increasing), but the only worry is that someone somewhere has written an application which somehow makes use of this error (by design or accident). Joe: how did you encounter this problem? ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2003-12-02 17:45 Message: Logged In: YES user_id=68433 Raising an error might be a good idea, but it would be backwards-incompatible. Tk 8.4 appears to use the width of a space as the minimum distance between chunks if the tab stop is too far to the left. ---------------------------------------------------------------------- Comment By: Vince Darley (vincentdarley) Date: 2003-12-02 15:55 Message: Logged In: YES user_id=32170 What do you suggest the correct behaviour is? An error message? Silently ignoring any smaller tabs? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=112997&aid=852949&group_id=12997 |