msg49086 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2005-11-21 18:06 |
This is a second patch for the text indentation - the first one was sent by mail and patched directly by KBK. I've touched up the indentation code to be cleaner and more generic. There's no longer need for a pad Frame, the padding is done by the Label widget. More IDLE patches coming - stay tuned ;) |
|
|
msg49087 - (view) |
Author: Kurt B. Kaiser (kbk) *  |
Date: 2005-11-23 01:25 |
Logged In: YES user_id=149084 At first glance the new code seems harder to understand and is longer. What is the advantage of going through the effort to apply, test, check in, and properly document the change? p.s. use Expand=False |
|
|
msg49088 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2005-11-23 09:04 |
Logged In: YES user_id=1330769 This patch is an improvement for two reasons: 1. One less TK widget is used 2. Replaces literal values with code that fetches the appropriate values from the editor window's widgets I'll admit it's not that big a deal. If it's really a lot of work, I'll leave it up to you to decide whether it's worth it. |
|
|
msg49089 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2006-11-19 19:18 |
The patch is incorrect: without the patch, the scrollbar for the text window ends below the context window. With the patch, the scrollbar is on the side of the context window also. Tentatively rejecting the patch; if you revise it, please add some comments explaining all the computations. |
|
|
msg49090 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2006-11-20 19:06 |
Seems like a bug in EditorWindow.py to me - shouldn't edtiwin.vbar (the scrollbar) be created inside editwin.text_frame instead of editwin.top? Seems to me that this is the reason text_frame is there in the first place - to encapsulate the Text widget and the scrollbar that accompanies it. Funny that I hadn't noticed the odd placement of the scrollbar... Thanks for the thorough review! I've submitted a revised patch, which fixes the bug in EditorWindow.py. I've also wrapped all widget attribute lookups in try/except blocks to avoid weird Tk behavior. This way future changes in Tk's inner workings will cause the text to be misaligned, instead of crashing IDLE. Lastly, I've added verbose comments where appropriate. Testing on current SVN trunk version of IDLE with only these changes shows no problems, and it works smoothly with my development version of IDLE as well. Let's finally get this piece of code behind us :) |
|
|
msg49091 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2006-11-22 08:51 |
Thanks for the patch. I can't find anything wrong anymore, and agree that the restructuring is better, so I have committed it as r52821. |
|
|
msg49092 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2006-11-22 14:30 |
Woohoo! :) |
|
|
msg49093 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2006-12-16 22:35 |
Revised excessively verbose comments, removed overly defensive try/except blocks, some minor comment and code cleanup. File Added: CodeContext.061217.patch |
|
|
msg49094 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2006-12-16 23:01 |
Hmm. This patch is already closed. Should the previous checkin be reverted? If this addresses additional issues, please submit it as a new report. |
|
|
msg49095 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2006-12-16 23:40 |
Revised excessively verbose comments, removed overly defensive try/except blocks, some minor comment and code cleanup. File Added: CodeContext.061217.patch |
|
|
msg49096 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2006-12-17 10:00 |
The changes were at Kurt's request. No new issues are addressed, it's only a revision of the previous changes. |
|
|
msg49097 - (view) |
Author: Kurt B. Kaiser (kbk) *  |
Date: 2007-02-06 19:12 |
Rev 53649 Thanks for the revised patch! |
|
|