Skip to content
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

IDLE editor line numbers #61737

Closed
rovitotv mannequin opened this issue Mar 24, 2013 · 59 comments
Closed

IDLE editor line numbers #61737

rovitotv mannequin opened this issue Mar 24, 2013 · 59 comments
Assignees
Labels
3.7 3.8 3.9 expert-IDLE type-feature

Comments

@rovitotv
Copy link
Mannequin

@rovitotv rovitotv mannequin commented Mar 24, 2013

BPO 17535
Nosy @rhettinger, @terryjreedy, @taleinat, @ned-deily, @serwy, @roseman, @rovitotv, @serhiy-storchaka, @animalize, @miss-islington, @aeros
PRs
  • #14030
  • #14916
  • #14917
  • #14959
  • #14973
  • #14974
  • Dependencies
  • bpo-3068: IDLE - Add an extension configuration dialog
  • Files
  • 17535IDLELineNumbers3dot4.patch
  • line-numbering-mockup.py
  • line numbering mockup image.png
  • line-numbering-v1.diff
  • line-numbering-v2.diff
  • linenumber-text-widget-v1.diff
  • tkfontsize.py: Test set commands from editor after font change.
  • linenumber-text-widget-v2.diff
  • linenumber-text-widget-v3.diff
  • 1.png
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/terryjreedy'
    closed_at = <Date 2019-07-23.13:35:22.751>
    created_at = <Date 2013-03-24.05:13:33.676>
    labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7', '3.9']
    title = 'IDLE editor line numbers'
    updated_at = <Date 2019-07-27.03:46:56.205>
    user = 'https://github.com/rovitotv'

    bugs.python.org fields:

    activity = <Date 2019-07-27.03:46:56.205>
    actor = 'miss-islington'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2019-07-23.13:35:22.751>
    closer = 'taleinat'
    components = ['IDLE']
    creation = <Date 2013-03-24.05:13:33.676>
    creator = 'Todd.Rovito'
    dependencies = ['3068']
    files = ['29567', '34395', '34397', '35689', '35723', '36194', '36285', '36384', '36387', '48455']
    hgrepos = []
    issue_num = 17535
    keywords = ['patch']
    message_count = 59.0
    messages = ['185115', '185164', '185166', '185170', '185173', '185181', '185543', '185805', '213386', '220973', '221256', '221321', '221341', '223086', '224504', '224911', '225377', '225379', '225384', '225387', '225402', '225411', '225416', '225418', '225419', '226048', '226372', '229709', '248131', '302325', '302340', '324885', '327602', '345417', '345418', '347184', '347238', '347257', '347258', '347281', '347297', '347403', '347702', '348266', '348267', '348331', '348332', '348333', '348334', '348360', '348372', '348469', '348472', '348473', '348479', '348491', '348537', '348538', '348539']
    nosy_count = 17.0
    nosy_names = ['rhettinger', 'terry.reedy', 'taleinat', 'ned.deily', 'roger.serwy', 'markroseman', 'jesstess', 'THRlWiTi', 'Todd.Rovito', 'serhiy.storchaka', 'JayKrish', 'Saimadhav.Heblikar', 'malin', 'Big Stone', 'miss-islington', 'mthompsonwhs', 'aeros']
    pr_nums = ['14030', '14916', '14917', '14959', '14973', '14974']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue17535'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    @rovitotv
    Copy link
    Mannequin Author

    @rovitotv rovitotv mannequin commented Mar 24, 2013

    I think it could be very helpful to add line numbers along the left side of the editor window. The feature could be toggled on/off easily enough. This was mentioned in the "Invent with Python" blog about IDLE so obviously other people would like the feature.

    @rovitotv rovitotv mannequin added expert-IDLE type-feature labels Mar 24, 2013
    @rovitotv
    Copy link
    Mannequin Author

    @rovitotv rovitotv mannequin commented Mar 24, 2013

    I got the extension from Roger Serwy's IDLEX, it is one of my favorite extensions. In addition to adding the extension I updated the documentation both idle.rst and help.txt. Finally I tested the patch on Mac OS X and it works great. This patch is for 3.4 but it does work with 2.7 but the documentation for 2.7 is not synced. Thanks.

    @rovitotv
    Copy link
    Mannequin Author

    @rovitotv rovitotv mannequin commented Mar 24, 2013

    For this patch to work correctly the option menu must be present so bpo-17532 http://bugs.python.org/issue17532 has to be resolved.

    @serwy
    Copy link
    Mannequin

    @serwy serwy mannequin commented Mar 24, 2013

    Todd, the LineNumbers.py extension from the IdleX project contains
    work-arounds for interacting cleanly with the Code Context extension. It
    also has a hack for dealing with the shortcomings of the
    Percolator/Delegator ordering.

    There are other shortcomings in the extension that I could not address
    from IDLE's extension loading architecture. Notably, IDLE does not have
    a method to broadcast that a font was changed, so the extension polls
    every second to see what the font is. The Code Context extension also
    does this. This is clearly an inefficient way to handle identifying a
    configuration change.

    I released IdleX with the University of Illinois. Presently, the code is
    under the NCSA license agreement and assigned to the Board of Trustees.
    If the PSF would allow inclusion as-is, then I'm all for it. Otherwise I
    will need to ask UIUC about how to re-license the code.

    @rovitotv
    Copy link
    Mannequin Author

    @rovitotv rovitotv mannequin commented Mar 25, 2013

    The NCSA license is very permissive I would be surprised if the PSF didn't accept it since both are BSD based. Needless to say I am not a lawyer and I am not sure who to speak with about this issue.

    I was able to find some precedence with the PEP-3146 which proposed the merging of Unladen Swallow with CPython. Unladen Swallow used LLVM which also used the NCSA license. But the merge never happened so I don't know what to think.

    Does this mean all the extensions from IDLEX are under NCSA license even Terminal.py?

    I am flexible please let me know how you want to proceed....

    @serwy
    Copy link
    Mannequin

    @serwy serwy mannequin commented Mar 25, 2013

    The file uploaded in 2010 falls under my PSF contributor agreement which
    has the Apache V2.0 license. The updates to the code in the latest
    version of IdleX fall under the NCSA license.

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Mar 30, 2013

    PSF accepts contributions from authors under Contributor Agreements. It does not grab software from 3rd parties, no matter how lenient the license. Roger, if you assigned all rights to the University, you should talk with them about either getting some back or about the University signing a corporate agreement. Also, there is a new psf 'legal' list for discussing legal matters.

    And yes, I am interested in line numbers also. I do not need them to edit my own files, but they are handy when communicating about stdlib files.

    @serwy
    Copy link
    Mannequin

    @serwy serwy mannequin commented Apr 2, 2013

    I received permission from UIUC to relicense IdleX code used for contributions into Python.

    @SaimadhavHeblikar
    Copy link
    Mannequin

    @SaimadhavHeblikar SaimadhavHeblikar mannequin commented Mar 13, 2014

    As a part of my GSOC 2014 proposal, i had prepared a mockup which adds linenumbering feature to a text widget.I am adding the mockup along with a image.

    Working:
    Intercepts low level calls and detects if any of the actions modify current view.If they do , then re-render the line numbers.Uses a canvas to display,(using create_text)

    *Also contains code to add breakpoints.Not a part of this issue,so ignore that aspect

    @SaimadhavHeblikar
    Copy link
    Mannequin

    @SaimadhavHeblikar SaimadhavHeblikar mannequin commented Jun 19, 2014

    Attached is a patch which adds linenumbering to IDLE. [1] is the current discussion regarding this topic at idle-dev.

    This patch is a initial patch. It is missing menu and config additions. I have posted it in this state, so that we can catch platform specific bugs and performance related issues(if any). In the patch, all major additions are in a new file LineNumber.py. This is keeping easier debugging in mind. The code will be restructured in the next version of the patch, which will have the above said additions and performance optimization(if any).

    I will be working on menu additions, config dialog additions and performance optimization in the mean time.

    For those who are interested, I used tk.call(self.text, 'dlineinfo', '%d.0' % linenum) instead of text.dlineinfo('%d.0' % linenum), because using any text.* method, used to cause a continuous increase in memory usage. I found this out the hard way, when, earlier I was making repeated text.index() calls.

    ---
    [1] - https://mail.python.org/pipermail/idle-dev/2014-June/003456.html

    @SaimadhavHeblikar
    Copy link
    Mannequin

    @SaimadhavHeblikar SaimadhavHeblikar mannequin commented Jun 22, 2014

    List of additions/changes
    1. EditorWindow uses LineNumber.Text instead of tkinter.Text.
    2. Added linenumber canvas to IDLE windows except PyShell
    3. Some info about LineNumber.Text
    a) Inherits tk.Text
    b) Generates <<Changed>> virtual event, when insert, delete, replace,
    cursor changes position, window resized, scrolled etc. Nothing else
    is affected. The result of the original call is returned.
    4. LineNumber.LineNumberCanvas info
    a) font_color and breakpoint_color have default values
    b) Linenumber and breakpoints disabled by default. Instantiating
    a LineNumberCanvas object does not pack it.
    c) Pack it programmatically by calling its "attach" method.
    Compulsorily supply the text widget to attach to.
    Supply other parameters like font_color, background,
    breakpoint_color as necessary.
    d) Unpack using "detach" method.
    e) Breakpoint feature is enabled only if LineNumberCanvas can
    "see" an EditorWindow instance called "editwin" as its attribute.
    It(editwin) should implement set_breakpoint(linenumber) and
    clear_breakpoint(linenumber) methods. EditorWindow responsible
    for binding breakpoint action on the canvas to LineNumberCanvas'
    toggle_breakpoint method.
    f) Contains a htest for GUI testing linenumbering and breakpoints.
    g) Any valid color can be set for linenumber canvas' background,
    its font color and breakpoint color. NS: Breakpoint does not have
    a background color.
    h) Linenumber canvas enabled by default.
    5. Linenumber preferences in configDialog's "General" tab.
    Highlight preferences in configDialog's "Highlight" tab.
    6. Other changes
    a) Refactoring of PyShell's set_breakpoint_here, set_breakpoint,
    clear_breakpoint_here and clear_breakpoint to make more consistent.

    @rhettinger
    Copy link
    Contributor

    @rhettinger rhettinger commented Jun 22, 2014

    FWIW, I don't think line numbers should be "enabled by default" for IDLE or any other editor. It isn't the norm and can be distracting. If you've ever tried to use IDLE to teach kids, you would value minimizing visual distractions.

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jun 23, 2014

    Many IDEs do show line numbers by default. And it does make discussing code with others simpler, e.g. when teaching. But I tend to agree with Raymond that it would be better to keep the default interface clean. Anyone who will want line numbers will be able to turn them on them easily.

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Jul 15, 2014

    I tried v2 and it works, subject to the following comments.

    1. (The biggest issue) For me, the 'obvious' implementation would be a narrow text window using the same font and size as the main window. The canvas introduces the problem of having to calculate the number position instead of letting tk do it for free. On my screen, they are two pixel too low.

    The fixed size for numbers can be a problem also. If someone with super eyesight (or a system that displays chars larger than nominal) uses 8 pt type, the bigger numbers are jammed together. If someone with poor eyesight uses a bigger font, the numbers will be too small. (This applies to dialogs too, but that is another issue.)

    Breakpoint symbols, for the few people who use them, could be selected from ascii ('|', '>', or '+') or non-ascii symbols. I do not think we need a canvas just to have a graphic for this.

    I am also interested in a marginal Text_strip class because I think one might be a possible solution to some of the problems with the Shell.

    1. Roger claimed "IDLE does not have a method to broadcast that a font was changed". Roger, as extension writer, had to take Idle as given, whereas we can add a 'font-changed' virtual event. This seems like a good idea. So would be a 'highlights-changed' event. It appears that editor windows get poked (notified) whenever the preferences Apply or Ok button are hit. Text is blackened and re-colorized even if there is no option change. (Edit a large file like EditorWindow.py to see this.) Adding this event should be a separate issue that this one depends on.

    2. To me, the default background for line numbers is way too dark. Easily changed.

    3. There should be a way to toggle line numbers in individual editor windows independently of the default setting for a user. (I agree that the delivered default should be off.) I would add this to the Format menu.

    @SaimadhavHeblikar
    Copy link
    Mannequin

    @SaimadhavHeblikar SaimadhavHeblikar mannequin commented Aug 1, 2014

    Attached is Text widget based implementation to add linenumbering to IDLE.

    NS: The purpose of comment block in update_sidebar_text_font
    The reason why it is there is to allow tk to "catch up" with changes(esp on large files) after a font change. While it *IS* not required for me, it *WAS* in the past. I don't know what has changed, but as it stands it is not required, IDK if it has been caused by a hardware change on my end.
    Anyways, while reviewing, please open a large file, and change the font size from minimum to maximum to minimum many times. Post here if the Linenumbering goes out of sync with and without uncommenting the comment block.

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Aug 6, 2014

    I believe the patch works slightly better on my system with the above mentioned block uncommented. The problem being addressed is this. The editor sendx a set command to the vertical scrollbar after *any* action that affects the lines displayed. We intercept the set command to set the sidebar in addition to the scrollbar slider. Works great. But after a font change, the editor emits two badly off and bogus commands, causing line numbers and the slider to unnecessarily jiggle up and down (or down and up). It then send a third set command with the proper fractions. Attached is the file I wrote to verify visual observations.

    I would like to commit this after checking a few details mentioned in previous messages. I would, however, like someone to first test the latest patch on OSX.

    @terryjreedy terryjreedy self-assigned this Aug 6, 2014
    @ned-deily
    Copy link
    Member

    @ned-deily ned-deily commented Aug 16, 2014

    As Terry requested, here are a few comments on running linenumber-text-widget-v1.diff on OS X. Overall, this looks to me to be a useful option.

    1. Having line numbering enabled by default was a bit of a surprise, particularly in light of 2.

    2. The "Toggle Linenumbering" menu item should be in the Options menu cascade, not the Window(s) one. I believe Options is where other extensions add their menu items. Initially, I could see no way to disable line numbering without knowing how to create, if necessary, and edit ~/.idlerc/config-extensions.cfg, which presumable few users do. Only later did I stumble across the "Toggle Linenumbering" menu option in the Window menu cascade. As an OS X user, I would never have thought to look there; normally on OS X, that menu cascade is limited to options related to selecting windows or tabs and selected options related to them, like Zoom or Minimize. (Presumably, the edit extensions preferences feature will help provide another, standard way for the user to deal with this in the long run.)

    3. I found the default color values made the line numbers difficult to read. Perhaps a better default would be fgcolor=White instead of Black when bgcolor=Gray?

    4. The extension fails to load with Tk 8.4.x:
      _tkinter.TclError: unknown option "-inactiveselectbackground"

    It appears that option is new in Tk 8.5:
    http://www.tcl.tk/man/tcl8.4/TkCmd/text.htm
    http://www.tcl.tk/man/tcl8.5/TkCmd/text.htm#M-inactiveselectbackground

    I'm not sure what to recommend here. Eventually, 8.4.x usage will slowly fade away so demanding that IDLE, especially extensions, not depend on any post-8.4 features may be a too-restrictive constraint. Perhaps it is OK to just test for pre-8.5 at extension initialization and cleanly skip if Tk is too old.

    1. The LineNumber extension should be added to the list of default extensions in the IDLE Help file (Lib/idlelib/help.txt) and the IDLE section of the Library Reference (Doc/library/idle.rst). Also add the "Toggle Linenumbering" menu item.

    2. The LineNumber extension config-extensions.def options need to be documented for the user (this is a generic issue for IDLE extensions). For example, I would not have had any clue that it was possible to enable line numbers for the shell window without examining the file and noticing the "enable_shell=0" hint.

    (Noted in passing: while the help/doc suggests: "See the beginning of config-extensions.def in the idlelib directory for further information.", even in the unlikely event that a user knew in what directory to look for it, it's not possible to open that file in an IDLE editor window with the current default Cocoa Tk's since Cocoa Tk does not provide a filter option on the open window; only .py* and .txt files can be opened.)

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Aug 16, 2014

    Thanks for the comments.

    1. I previously said somewhere 'enabled but initially off' by default. I intend to make sure of that before committing.

    2. Option menu sounds good..

    3. I tried out a *much* lighter gray, as with Notepad++. Saimadhav is leaving that to me to determin and change.

    4. If inactiveselectbackground is needed and there is no alternative (Saimadhav?) I would be inclined to skip pre-8.5.

    5. I have put off writing a doc until the spec is finished. (And I would like to have help.txt auto-generated from idle.rst, which is an issue somewhere.)

    6. I do not intend line numbering for Shell. Rather, the sidebar should be used for prompt and output markers to solve bpo-7676. The patch has been written with this in mind. How to handle this vis-a-vis config-extensions had not been decided yet. I would prefer one [sidebar] section, but Saimadhav does not think that is possible. In any case, we have focused first on just line numbering, which I would like to push with the revisions indicated above, and then adding breakpoint toggling (in a followup patch, separately reviewed) and the shell sidebar (in yet another patch, ditto).

    (Note. see bpo-22209)

    @SaimadhavHeblikar
    Copy link
    Mannequin

    @SaimadhavHeblikar SaimadhavHeblikar mannequin commented Aug 16, 2014

    @ Ned Deily:
    Thank you for the comment's.

    1. I documented it in the config-extensions.def how to make it visible on startup(or not). In the new patch, it is not visible by default.

    2. I have made the changes.

    3. I'll explain the reason why this argument is required:
      Without this argument, if a user "selects" the sidebar Text widget, a highlight can be seen around the selected area. Adding this argument prevents it showing up.
      The new patch prevents this unknown option error, by adding the inactiveselectbackground only if Tk version >= 8.5

    (Because of this issue which you raised, I found a new bug and fixed it - see below.)

    1. Done

    2. I added a comment in the config-extensions.def file. The enable_shell option has been removed for now. It is not related to linenumbering.

    @terry Reedy:

    1. Done. Disabled by default

    2. I mentioned the reason why it is required above.

    ------------
    New bug which was discovered because of 4 and fixed:
    Scrolling on the sidebar text widget using mouse by dragging would scroll just the sidebar text widget and not the main text widget. This has been fixed by adding self.text.yview_moveto(args[0]) in vbar_set.

    @ned-deily
    Copy link
    Member

    @ned-deily ned-deily commented Aug 16, 2014

    Thanks for addressing the comments. With linenumber-text-widget-v2.diff:

    1. With Tk 8.4, the extension gets a bit further but still fails:

    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/main.py", line 9, in <module>
    idlelib.PyShell.main()
    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/PyShell.py", line 1570, in main
    shell = flist.open_shell()
    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/PyShell.py", line 327, in open_shell
    if not self.pyshell.begin():
    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/PyShell.py", line 1054, in begin
    (sys.version, sys.platform, self.COPYRIGHT, nosub))
    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/PyShell.py", line 1296, in write
    count = OutputWindow.write(self, s, tags, "iomark")
    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/OutputWindow.py", line 40, in write
    self.text.insert(mark, s, tags)
    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/Percolator.py", line 25, in insert
    self.top.insert(index, chars, tags)
    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/LineNumber.py", line 81, in insert
    self.changed_callback(get_end(self.delegate))
    File "/Users/nad/Projects/PyDev/active/dev/3x/t/idlelib/LineNumber.py", line 158, in update_sidebar_text
    self.sidebar_text['width'] += width_difference
    TypeError: Can't convert 'int' object to str implicitly

    1. I just noticed that the (now) "Line Number" menu item has no visual indication of its state nor does it preserve its state over IDLE sessions. The Code Context extension for edit windows does both: the menu item has a checkmark when enabled and that state is restored when IDLE is restarted.

    @SaimadhavHeblikar
    Copy link
    Mannequin

    @SaimadhavHeblikar SaimadhavHeblikar mannequin commented Aug 16, 2014

    On 16 August 2014 14:03, Ned Deily <report@bugs.python.org> wrote:

    1. With Tk 8.4, the extension gets a bit further but still fails:

    I fixed this now. Please let me know how it works now.

    1. I just noticed that the (now) "Line Number" menu item has no visual indication of its state nor does it preserve its state over IDLE sessions. The Code Context extension for edit windows does both: the menu item has a checkmark when enabled and that state is restored when IDLE is restarted.

    Done.

    @ned-deily
    Copy link
    Member

    @ned-deily ned-deily commented Aug 16, 2014

    > 1. With Tk 8.4, the extension gets a bit further but still fails:
    I fixed this now. Please let me know how it works now.

    Yep, 8.4 now seems to work just fine.

    > 2. I just noticed that the (now) "Line Number" menu item has no visual indication of its state nor does it preserve its state over IDLE sessions. The Code Context extension for edit windows does both: the menu item has a checkmark when enabled and that state is restored when IDLE is restarted.
    Done.

    Thanks, that's much better. One small problem: the "Line Number" menu option appears to be a global one, affecting all new windows. But, if the option is toggled when more than one window is open (edit and/or shell), only the window which has the focus changes (adds or deletes the numbering sidebar); the other windows do not change although their "Line Number" menu option does (i.e. the check mark correctly appears or disappears). So now the other windows are "out-of-sync" WRT line number status and could lead the user to think that the "Line Number" option is supposed to be local to each window. Then when restarting IDLE, all windows are created with the last global value of "Line Number". If the option is supposed to be global, then, when toggling the menu option, all open windows should ideally be updated immediately or at least updated the next time each becomes active.

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Aug 16, 2014

    I believe the line number behavior Ned describes is the same as the Code Context behavior. I just tested the latter to verify Saimadhav's email report. I think both *should* be the same.

    The docs say Code Context is a toggle. However, it does not mention the checkmark or the initial state of new windows. Clicking Code Context toggles the state of the current window only; I think this is proper. I also want that for line-numbering.

    It also toggles the initial state of future windows, as indicated by the checkmark. In other words, the checkmark does NOT indicate the state of the current window, which is visually obvious anyway. Instead, it indicates the initial state of new windows, and it the latter that is saved across sessions. This behavior may not be expected, but given the alternatives*, I consider it plausible if not designed. It definitely needs to be documented.

    • So what are the alternatives?
    1. Make the checkmark toggle local to the window, as you expected. But then what would be the initial state of new windows (and saved as such)? One solution would be two Code Context options, one for current window (checkmark not really needed) and another for future windows (saved). The current design cleverly combines two buttons into one. Perhaps it is too clever, but I don't like two buttons either. Another solution would be to just use whatever is in config default+user. This would be more palatable once we add a config-extension dialog. But it would still be more of a nuisance than the current within-session switching.

    2. Make the window toggle global to all current windows. I suspect that this would be visually obnoxious. If someone hated Context/Numbering and only turned either on temporarily, intended for one window, out of necessity, it would be behaviorally obnoxious. There may also have been a performance consideration that is somewhat but not totally obsolete.

    The feature was added in bpo-936169 in 2005. AFAIK, there have been no complaints. There are none I could find on the tracker. So my inclination is to commit Line Numbering with the same menu behavior as Code Context. If we ever want to change (both), that should be a new issue and discussion. Tal, you were involved in bpo-936169. Any comments?

    @serhiy-storchaka
    Copy link
    Member

    @serhiy-storchaka serhiy-storchaka commented Aug 16, 2014

    1. Make the checkmark toggle local to the window and make the preferences
      option toggle default value to all windows. This is how it works in Kate (KDE
      text editor).

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Aug 16, 2014

    I believe your 3. is my 1b. It really requires being able to change preference for an extension without editing ~HOME/.idlerc/config-extensions.cfg by hand. See bpo-3068.

    @serwy
    Copy link
    Mannequin

    @serwy serwy mannequin commented Aug 29, 2014

    When it comes to the checkmark next to Code Context in the menu, be aware of bpo-13179. You can launch IDLE, open two separate editors, enable Code Context in one, and the other will have its menu entry checked as well when it is not enabled.

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Sep 4, 2014

    And if you enable code context in the other window, neither will have the checkmark. As I said, the one button is being used to toggle two things: the state of the current window and the default for future windows, and the checkmark only indicates the second. I agree that this is confusing.

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jun 12, 2019

    Please see updated PR, #58238.

    It's not 100% ready yet. At this point the goal is to have some people try it and give feedback. So please, give it a go and let me know what you think!

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jun 12, 2019

    In reference to previous discussion here about the effect of toggling line numbers on future opened windows: IMO toggling shouldn't affect future windows; those should behave according to the configured default state. The default state should be changed only by editing the config (whether via the config dialog or by editing the config files.)

    @taleinat taleinat added the 3.9 label Jun 12, 2019
    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 3, 2019

    I'm rather surprised at the lack of enthusiasm for this here and on idle-dev! I was sure people would be happy to see this finally approaching completion after so many years.

    To clarify, all I'm waiting for now is to hear whether this change is wanted, before I do the extra work to finalize it.

    @ned-deily
    Copy link
    Member

    @ned-deily ned-deily commented Jul 3, 2019

    Tal, I took a quick look at the results with the current PR and, as someone who doesn't use IDLE other than to smoke test, It looks fine to me. The concerns I raised previously about the interaction between changing the state of Options->Line Numbers and multiple windows still stands, though. Perhaps not previously mentioned, this may be an issue specific to macOS where, per macOS interface guidelines (and as provided by Tk), there is only one instance of a menu bar (at the top of the screen) per app, not a menu bar per app window. Also, since my original review comments, we have eliminated the use on macOS of Tk versions older than 8.6 so previous concerns are no longer relevant.

    @animalize
    Copy link
    Mannequin

    @animalize animalize mannequin commented Jul 4, 2019

    I tried PR 14030 today.
    By default, the fgcolor is black.
    Looks like a black belt always on the left side, this makes me feel a bit oppressive.
    Of course, the fgcolor can be changed.

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 4, 2019

    Ned, thanks for taking a look!

    If it is decided to go forward with this then I will make sure to make the menu item state consistent with that of each window.

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 4, 2019

    Looks like a black belt always on the left side, this makes me feel a bit oppressive.

    This currently uses the same colors as the code-context panes, which is configurable as the "context" fg/bg colors. We might find a better name for this, or even add a separate color configuration option, if this comes up as a common request (I have it feeling it will be).

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Jul 5, 2019

    Tal, I will be delighted to see this finally land. Please continue. Some immediate comments to update the years-ago discussion.

    1a. IDLE now explicitly requires tk 8.5. AFAIK, it is only tested on 8.6. 1b. Feature are no longer extensions.

    2a. The code context checkmark is gone, replaced by toggling the menu label between 'Show Code Context' and 'Hide Code Context'. The label only applies to to the current window. All windows start with the default of 'off', with label 'Show'. A global setting could be added to make the default on, but AFAIK this has not been requested, and I prefer not adding global options unless *really* needed.

    2b. Zoom Height, default off, has been moved from Window to the Options menu, below ? Code Context. It had no checkmark and was never global. It now has a label toggle, Zoom versus Restore.

    2c. The Options menu separator line separates the global settings dialog from current-window-only options, with room for more. AFAIK, both work on macOS. I would like to follow the pattern with 'Show/Hide Line Numbers', with 'off' the default for new windows. We might put it above 'Show/Hide Code Context' as likely to be used more often. I would prefer to wait before possibly adding a global toggle.
    ---

    Notepad++ has dark gray on light gray numbers. I can see how bold black on white is a bit much. But I want to focus on behavior next.

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 5, 2019

    I've made some significant improvements in several aspects:

    1. Scrolling: performance and behavior
    2. Selection entire lines by dragging over line numbers
    3. Color config: a separate highlighting setting for line numbers, which takes effect immediately

    I've also remove the Tk version >= 8.5 check, and replaced the hack for getting font updates to take immediate effect.

    As far as I can see, the only things left to resolve are:

    1. Clear, consistent behavior regarding whether line numbers are enabled by default, and when and how this default is changed.
    2. Menu item location, type (boolean flag?) and correctly reflecting the state of each window.

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 11, 2019

    To those following this issue, PR #58238 is now in a very good state, and would benefit from more feedback.

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Jul 21, 2019

    Tal's 2 issues above have been resolved.

    1. Line numbers are initially off by default but this can be reversed on the Settings General tab.
    2. Line numbers can be shown and hidden for a window on the Option menu.

    The only bug I know of that must be fixed before merging is that font changes do not always get propagated to the numbers.

    I recently added 1 pixel of padding to line up line numbers and text. How is is on other machines? Especially Mac, if anyone has one? (Tal's just broke.) Easy test: open a new file; type multiple 2s and 7s on lines 2 and 7. Eyeball, then, use paper to see if low and high horizontal lines on 2s and 7s are lined up properly.

    @terryjreedy terryjreedy changed the title IDLE: Add an option to show line numbers along the left side of the editor window, and have it enabled by default. IDLE editor line numbers Jul 21, 2019
    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Jul 21, 2019

    Ned, about window menus and Mac's single menu: tk and macOS work together to switch the menu to the menu of a window that becomes active. I opened two editor windows and clicked 'Show Code Context' in just one of them. The menu then said 'Hide Code Context'. I clicked the other window and then Option and it said 'Show...'. More experiments clicking windows and toggling CC went well. Show/Hide Line Numbers is intended to work the same way. Since Tal cannot now test, verification would be nice, as well as the line-up check described above

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 23, 2019

    New changeset 7123ea0 by Tal Einat in branch 'master':
    bpo-17535: IDLE editor line numbers (GH-14030)
    7123ea0

    @miss-islington
    Copy link
    Contributor

    @miss-islington miss-islington commented Jul 23, 2019

    New changeset 1da6a31 by Miss Islington (bot) in branch '3.8':
    bpo-17535: IDLE editor line numbers (GH-14030)
    1da6a31

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 23, 2019

    New changeset e9ec166 by Tal Einat in branch '3.7':
    [3.7] bpo-17535: IDLE editor line numbers (GH-14030)
    e9ec166

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 23, 2019

    PR Merged! This will be in 3.8.0 and 3.7.5. Thanks to everyone who worked on this and took part in the discussion!

    Special thanks to Saimadhav for preparing a patch and iterating on it several times; I picked up the work from where he left off, and it was a great starting point.

    Special thanks also to Terry for stewarding the development of IDLE in general, and for working with me actively on the PR for this over the past month and a half.

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Jul 24, 2019

    A note on the first 3 comments in msg223086 of 2014-07-15.

    1. Canvas versus Text: The prototype posted by Brian Oakley on SO had to use a Canvas because it was meant to work even with texts with multiple fonts and in-line widgets. IDLE's editor, intended mainly for .py files, does not allow either. As long as line numbers use the same font as the text, they have the same height and vertical spacing as the text.

    I checked that this remains true when non-latin character sets are mixed in. I pasted the config font sample into a numbered editor and it worked perfectly. At least on Windows, tk fonts are vertically monospaced at least for this sample, and I assume for all. To do this, either ascii/latin/etc lines are over-spaced or hanji/kanji/hangul lines are underspaced. Source Code Pro, which I normally use, does the former. Courier does the latter.

    1. Call versus Event notification. IDLE's menu and hot keys generate pseudoevents handled by one event handler. Some tk widgets generate widget-specific pseudoevent that can be handled by any handler. IDLE's config dialog pushes font change notifications by calling text methods that amount to event handlers. It can do this because 'listed windows' are registered as listeners by being on the list of such instances.

    I suggested 5 years ago that we might switch font change notification to events. I don't think so now. The current system allows control of the order in which texts within a code frame respond to config changes. It works better if code contexts, when present, change before the main text.

    1. Line number foreground and background. We made them configurable to user preference. Possibilities are: same as text; toned down version of text (for instance, black on white to dark gray on light gray), or contrasting with text.

    (I discussed 4. global versus local setting on 7/21.)

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Jul 24, 2019

    After-thought test 2: proportional fonts* work fine. Nearly all have monospaced digits, with extra space around '1' if there is no bottom bar. So those also have nicely aligned right-justified columns in the sidebar. And the one font I found with proportional digits, Miriam Libre, still had readable numbers.

    • Given the PEP-8 discouragement of the sort of vertical alignment, such as
    one      =  1
    eight    =  8
    ten      = 10
    nineteen = 19

    which people, including me, have done with other languages, using a proportional font with python is not nearly as crazy as some people claim, and not to be totally disregarded. The main issue is line length and what happens when someone else views the same file with a fixed-pitch font.

    @aeros
    Copy link
    Member

    @aeros aeros commented Jul 26, 2019

    After testing it on Linux (through the latest dev version, launched with ./.python -m idlelib), the line numbers seem to be working quite well across several different fonts and font sizes. My only suggestion for minor visual improvement would be to increase the margin between the right side of the number and the line.

    While moving through a number of different fonts and sizes, I noticed a separate issue involving the window scaling based on the size of the font previews. This isn't an issue for smaller fonts, but when attempting to use larger fonts (upper 20s or into the 30s depending on the font in question) the 4 buttons ("Ok", "Apply", "Cancel", and "Help") in the settings menu (options > configure IDLE) are moved past the lower end of the screen, making them no longer visible to the user.

    Either way, this change is pretty awesome. The only thing preventing me from using the IDLE more was the lack of line numbers, so I'll definitely be using it a lot more now. Thanks for the PR taleinat!

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 26, 2019

    Thanks for trying this and giving such useful feedback, Kyle!

    Interestingly, Terry just opened an issue about exactly what you mention regarding the font configuration window; see issue bpo-37628.

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 26, 2019

    Correction: Terry commented on that issue (bpo-37628) earlier today; Raymond Hettinger originally reported it a about a week ago.

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Jul 26, 2019

    The current right margin is 1 pixel. Let's try making it 2.

    @taleinat
    Copy link
    Contributor

    @taleinat taleinat commented Jul 26, 2019

    The current right margin is 1 pixel. Let's try making it 2.

    See PR #59164. I added 2 pixels of horizontal padding to the line numbers text widget, and it certainly does look much nicer. Thanks for the suggestion, Kyle!

    @terryjreedy
    Copy link
    Member

    @terryjreedy terryjreedy commented Jul 27, 2019

    New changeset 46ebd4a by Terry Jan Reedy (Tal Einat) in branch 'master':
    bpo-17535: Increase line number horizontal padding by 2 pixels (GH-14959)
    46ebd4a

    @miss-islington
    Copy link
    Contributor

    @miss-islington miss-islington commented Jul 27, 2019

    New changeset 9e7697b by Miss Islington (bot) in branch '3.7':
    bpo-17535: Increase line number horizontal padding by 2 pixels (GH-14959)
    9e7697b

    @miss-islington
    Copy link
    Contributor

    @miss-islington miss-islington commented Jul 27, 2019

    New changeset f6ab188 by Miss Islington (bot) in branch '3.8':
    bpo-17535: Increase line number horizontal padding by 2 pixels (GH-14959)
    f6ab188

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 3.8 3.9 expert-IDLE type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants