[Patches] [ python-Patches-1362975 ] CodeContext - Improved text indentation

SourceForge.net noreply at sourceforge.net
Sun Dec 17 00:01:41 CET 2006


Patches item #1362975, was opened at 2005-11-21 19:06
Message generated for change (Comment added) made by loewis
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1362975&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: IDLE
Group: None
Status: Closed
Resolution: Accepted
Priority: 3
Private: No
Submitted By: Tal Einat (taleinat)
Assigned to: Kurt B. Kaiser (kbk)
Summary: CodeContext - Improved text indentation

Initial Comment:
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 ;)

----------------------------------------------------------------------

>Comment By: Martin v. Löwis (loewis)
Date: 2006-12-17 00:01

Message:
Logged In: YES 
user_id=21627
Originator: NO

Hmm. This patch is already closed. Should the previous checkin be
reverted?

If this addresses additional issues, please submit it as a new report.

----------------------------------------------------------------------

Comment By: Tal Einat (taleinat)
Date: 2006-12-16 23:35

Message:
Logged In: YES 
user_id=1330769
Originator: YES

Revised excessively verbose comments, removed overly defensive try/except
blocks, some minor comment and code cleanup.
File Added: CodeContext.061217.patch

----------------------------------------------------------------------

Comment By: Tal Einat (taleinat)
Date: 2006-11-22 15:30

Message:
Logged In: YES 
user_id=1330769
Originator: YES

Woohoo! :)

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2006-11-22 09:51

Message:
Logged In: YES 
user_id=21627
Originator: NO

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.

----------------------------------------------------------------------

Comment By: Tal Einat (taleinat)
Date: 2006-11-20 20:06

Message:
Logged In: YES 
user_id=1330769
Originator: YES

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 :)

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2006-11-19 20:18

Message:
Logged In: YES 
user_id=21627
Originator: NO

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.

----------------------------------------------------------------------

Comment By: Tal Einat (taleinat)
Date: 2005-11-23 10:04

Message:
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.

----------------------------------------------------------------------

Comment By: Kurt B. Kaiser (kbk)
Date: 2005-11-23 02:25

Message:
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

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1362975&group_id=5470


More information about the Patches mailing list