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: Make multiple improvements to CodeContext #77791

Closed
terryjreedy opened this issue May 23, 2018 · 27 comments
Closed

IDLE: Make multiple improvements to CodeContext #77791

terryjreedy opened this issue May 23, 2018 · 27 comments
Assignees
Labels
3.10 only security fixes topic-IDLE type-feature A feature request or enhancement

Comments

@terryjreedy
Copy link
Member

BPO 33610
Nosy @terryjreedy, @taleinat, @csabella, @miss-islington
PRs
  • bpo-33610: Update IDLE Code Context doc entry #7597
  • [3.7] bpo-33610: Update IDLE Code Context doc entry (GH-7597) #7598
  • [3.6] bpo-33610: Update IDLE Code Context doc entry (GH-7597) #7599
  • bpo-33610: When toggling code-context on, immediately show the current context. #14821
  • bpo-33610: validate non-negative integer inputs in IDLE's config #14822
  • [3.8] bpo-33610: IDLE's code-context always shows current context immediately (GH-14821) #14846
  • [3.7] bpo-33610: IDLE's code-context always shows current context immediately (GH-14821) #14847
  • [3.8] bpo-33610: validate non-negative integer inputs in IDLE's config (GH-14822) #14913
  • [3.7] bpo-33610: validate non-negative integer inputs in IDLE's config (GH-14822) #14914
  • bpo-33610: Edit idlelib.codecontext #23773
  • [3.8] bpo-33610: Edit idlelib.codecontext (GH-23773) #23774
  • [3.9] bpo-33610: Edit idlelib.codecontext (GH-23773) #23775
  • Dependencies
  • bpo-22703: Idle Code Context menu entrie(s)
  • bpo-31493: IDLE cond context: fix code update and font update timers
  • bpo-32831: IDLE: Add docstrings and tests for codecontext
  • bpo-33628: IDLE: Code cleanup on codecontext
  • bpo-33642: IDLE: Display up to maxlines non-blank lines for Code Context
  • bpo-33664: IDLE: scroll text by lines, not pixels.
  • bpo-33763: IDLE: Use text widget for code context instead of label widget
  • bpo-35521: IDLE: Add doc section for Code Context and ref links.
  • bpo-35555: IDLE: Gray out Code Context on non-editor windows
  • bpo-37530: IDLE: simplify, optimize, and clean up code context
  • 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 = None
    created_at = <Date 2018-05-23.05:18:11.915>
    labels = ['expert-IDLE', 'type-feature', '3.10']
    title = 'IDLE: Make multiple improvements to CodeContext'
    updated_at = <Date 2020-12-15.12:52:04.350>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2020-12-15.12:52:04.350>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = False
    closed_date = None
    closer = None
    components = ['IDLE']
    creation = <Date 2018-05-23.05:18:11.915>
    creator = 'terry.reedy'
    dependencies = ['22703', '31493', '32831', '33628', '33642', '33664', '33763', '35521', '35555', '37530']
    files = []
    hgrepos = []
    issue_num = 33610
    keywords = ['patch', '3.5regression']
    message_count = 39.0
    messages = ['317357', '317364', '317480', '317599', '317601', '317651', '317822', '318012', '318351', '318355', '318357', '318358', '318466', '318476', '318562', '318589', '318791', '319240', '319241', '319242', '319243', '332034', '332105', '332312', '347571', '348001', '348078', '348081', '348125', '348126', '348127', '348324', '348325', '348326', '383031', '383033', '383035', '383036', '383048']
    nosy_count = 4.0
    nosy_names = ['terry.reedy', 'taleinat', 'cheryl.sabella', 'miss-islington']
    pr_nums = ['7597', '7598', '7599', '14821', '14822', '14846', '14847', '14913', '14914', '23773', '23774', '23775']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue33610'
    versions = ['Python 3.10']

    @terryjreedy
    Copy link
    Member Author

    terryjreedy commented May 23, 2018

    [After move to GH issues, I edited multiple comments into task list and deleted comments made obsolete.]
    There are a number of possible improvements to CodeContext. They be separate issues (and PRs) that are dependencies of this master index issue. Some should be fairly easy now that we have the new tests.

    • 1. IDLE: Add docstrings and tests for codecontext #77012 Add docstrings and tests. Review has notes.
    • 2. IDLE: Code cleanup on codecontext #77809 Follow-up may revise and do some user-invisible code cleanups.
    • 3. IDLE cond context: fix code update and font update timers #75674 Cancel event loops when instances are deleted.
    • 4. Spinoff from above: 1 or 2 events loops for class, not each instance. [Replaced with 15 and 16.]
    • 5. Idle Code Context menu entrie(s) #66892 Separate initial context state of new window from toggling
      the state of current windows. Current behavior is buggy.
    • 6. Gray-out Options => Code Context on non-editor windows.
      (Must revise again if windows became panes in master window.)
    • 7. IDLE: Display up to maxlines non-blank lines for Code Context #77823 Change fixed # of lines to variable # of lines as needed, up to limit. About 15 is limit for 4-space indents in 80 char lines.
    • 8. Click on context line jumps to line. Show it at top of window.
      Implement with text.xview_scroll(target_topline - current_topline, 'units').
    • 9. IDLE: Enable theme-specific color configuration for code context #77860 Reenable user config of context colors. Add the current black on gray to the light theme
      and appropriate values, to be determined, to the dark theme.
    • 10. Somehow mark blocks in editor. Subtle background color change? [14 is more feasible.]
      Or tag just the indents, or, if add line numbers, the line numbers? [Rep
    • 11. IDLE: Use text widget for code context instead of label widget #77944 Use read-only Text instead of Label for context. Text.index(f'@{e.x},{e.y}', where e is event,
      converts mouse click to line number. That can be mapped to text line number for 8. above. Text also enables
    • 12. If context box is too short for all lines, add scrollbar.
    • 13. When horizontal scrollbars are added to editor, scroll context along with text scroll.
    • 14. Block structure navigation: As near as I can tell, Alt-arrowkey is currently the same as arrowkey. Have Alt-Up and Alt-Down move to the next block line above or below with the same indent and Alt-Left, Alt-Right move up and out (smaller indent) or down and in (larger indent). [More feasible than 10.]
    • 15. (Instead of 4b. One font loop for class.) Replace font loop with font-change notification by the editor when the editor gets such a notification (which is very rare).
    • 16. (Instead of 4a. One font loop for class.) The current loops update every window 10 times a second, which is about the upper limit for a great typist or normal scroller. But only an active editor window needs updating, and there is at most 1. Most checks are wasted.
    • 16A. Replace update loop with notification of visible display changes. Pro: We already monitor key and mouse actions. Anti: the event loop is simple.
    • 16B. Only run update loop in active window. Anti: I don't think we currently monitor window focus. Pro: if we don't, we will need to for a multipane, multitab IDLE. (Stopping should be easy: check status before after call in callback. This automatically does a last update after focus gone.)
    • 17. IDLE: scroll text by lines, not pixels. #77845 Scroll by lines instead of sometimes by pixels.
    • 18. Error or bounds checking for maxlines entry. I believe this really a config dialog issue that applies to everything, but maxlines should be at least 1, with some reasonable max. In the meanwhile, we could check whether crazy entries (text, negatives, super high) can crash IDLE.
    • 19. bpo-33610: Update IDLE Code Context doc entry #7597 idle.rst doc change
    • 20. What's New for 3.6.6 and 3.7.0: These could be patches on this issue.
    • 21. idlelib/NEWS.txt entries

    Undone items added to new task list below. #77791 (comment)

    @terryjreedy terryjreedy self-assigned this May 23, 2018
    @terryjreedy terryjreedy added topic-IDLE type-feature A feature request or enhancement 3.7 (EOL) end of life 3.8 only security fixes labels May 23, 2018
    @terryjreedy terryjreedy changed the title IDLE: Improve CodeContext (various issues) IDLE: Make multiple improvements to CodeContext May 23, 2018
    @terryjreedy
    Copy link
    Member Author

    terryjreedy commented May 24, 2018

    With 2. merged, I consider the next priorities to be 5. 66892 (overt bug, highest), 7, and 8, to make the feature more usable.

    1. is a vague speculative feature that would use get_indent_firstword but would actually be for the editor. Update problem: It requires a representation of the structure of the entire code, and any edit could change that. The saving grace of code context is that it only maintains information about the invisible lines above the cursor.

    @csabella
    Copy link
    Contributor

    csabella commented May 24, 2018

    I've started looking at 5. bpo-22703, more specifically, I've been trying to recreate it. Since the config changes in 3.6/3.7 where the flag was removed from config dialog, the code context needs to be turned on explicitly for each editor. 3.5 has the old behavior.

    So, I think the current change would be to add it back to config but with the behavior defined in bpo-22703.

    I hope to have a PR within the next few days.

    @terryjreedy
    Copy link
    Member Author

    While trying out variable lines, bpo-33642, I encounter two related issues.

    First is the context colors. Black on gray is a variant of black on white. I am using the dark theme, white on dark blue. For fixed sized context, the high contrast is okay. For variable context, having lines flip from light on dark to dark on light and back is distracting to obnoxious. So I want to change
    9. Reenable user config of context colors?
    to
    9. Reenable user config of context colors!
    Add the current black on gray to the light theme and appropriate values, to be determined, to the dark theme.

    Second is getting partial lines at the top of the text box as a result of scrolling with the mousewheel or scrollbar slider. I never liked this but tolerated it. With complete text lines above, it looks awful to me. Hence, I consider the following to be a dependency of this issue.
    17. bpo-33664 Scroll by lines instead of sometimes by pixels.

    Following the implementation notes for bpo-33664, once the label is changes to text, to get the target topline, 8. can be implemented with
    text.xview_scroll(target_topline - current_topline, 'units')

    I would like to release multiple user visible improvements at once, at least 7, 9, and 17, so the new code context looks good from the start. 8, menu changes, and 14 could come later.

    @terryjreedy
    Copy link
    Member Author

    The release candidate has been re-scheduled for June 11. So we should be able to merge the minimal upgrade before that and get it in 3.7.0 and 3.6.6. (The latter will come out at the same time as the former.)

    Variable lines is about ready. Colors looks like it should be, with a final test needed after variable lines is merged (or maybe I will reverse the order). That leaves line alignment. If you are working on it, but don't have a PR ready yet, please say so.

    @terryjreedy
    Copy link
    Member Author

    5, reformulated. Now that 'Code Context' on the Options menu only toggles a feature for the current window, like 'Zoom Height' on the Window menu, both should appear together on the same menu. My current inclination is to move 'Code Context' to Window because a) it is the feature be that will be changed and discussed in What's New, and b) 'Window' otherwise lists individual windows so it more clearly implies that toggles are for the current window without adding a fake menu entry. Agree?

    @csabella
    Copy link
    Contributor

    csabella commented Jun 1, 2018

    That leaves line alignment. If you are working on it, but don't have a PR ready yet, please say so.

    I was going to work on it this weekend, but haven't started yet.

    I agree that moving Code Context to the Windows menu makes sense.

    @terryjreedy
    Copy link
    Member Author

    I merged 'variable lines' and edited 'colors' to fix the resulting conflicts. Travis passed the latter so I expect to merge it also. To avoid more conflicts, you should wait to create the 'alignment' branch until 'colors' is merged and pulled into your clone. (We should continue to keep having two pending patches for the same file rare.)

    Counter-argument for menu location. Window has 1 to n top-level window items. Options will have just one item without Code Context. So moving Zoom would make things more balanced.

    @terryjreedy
    Copy link
    Member Author

    After writing the above, I realized that line scroll only touches editor.py, not codecontext.py. On the other hand, the next item, 8. jump to context line did have to wait. Replacing Label with Text will require explicit height setting with each context change. The click handler will be a simplified version of the scroll handler you just wrote. Jumping to a line will look much better when it jumps to a whole rather than a fractional line. Again, one of us should say something before starting.

    @csabella
    Copy link
    Contributor

    csabella commented Jun 6, 2018

    Menu location:

    If Code Context is moved under Windows, maybe Configure IDLE should be moved as well?

    • VS Code has Preferences under the File menu.
    • Spyder has a menu option called Tools which contains Preferences, Update PYTHONPATH manager, and Reset Spyder to defaults
    • Atom has Preferences under the Edit menu.
    • Sublime Text has a menu called Preferences.
    • Notepad++ has a menu called Settings, which includes Preferences.

    @terryjreedy
    Copy link
    Member Author

    New changeset af4b013 by Terry Jan Reedy in branch 'master':
    bpo-33610: Update IDLE Code Context doc entry (GH-7597)
    af4b013

    @terryjreedy
    Copy link
    Member Author

    terryjreedy commented Jun 10, 2018

    General Update: We have done 1, 3, 7, 8, 9, 11, 17, 21. Very nice. A re-organized list of the remainder (with old numbers).

    Docs:

    Menu

    Code Context Display

    Internal Changes

    External Changes

    • E1. Mark blocks in editor (10, unlikely).
    • E2. Navigate by blocks in editor (14).
    • E3. bpo-33610: validate non-negative integer inputs in IDLE's config #14822 Error check maxlines in configdialog (18).
    • E4. Max context lines = 0 is treated as infinity. For depth 0, "self.info[-self.context_depth:]" is whole slice. I don't remember if this is intended (if one does not want context, don't toggle it on), but it is not documented in settings help or doc section. In any case, offset = max(1, lines - self.context_depth) - 1 fails with depth 0.
    • E5. test_codecontext is affected by user customization, but should not be.

    @miss-islington
    Copy link
    Contributor

    New changeset 2adfeef by Miss Islington (bot) in branch '3.7':
    bpo-33610: Update IDLE Code Context doc entry (GH-7597)
    2adfeef

    @miss-islington
    Copy link
    Contributor

    New changeset 08a1b13 by Miss Islington (bot) in branch '3.6':
    bpo-33610: Update IDLE Code Context doc entry (GH-7597)
    08a1b13

    @terryjreedy
    Copy link
    Member Author

    terryjreedy commented Jul 16, 2019

    I2: replace font/hightlight loop with notification and immediate change instead of a delay is a bugfix.

    In testing PR 14675 for bpo-37530 I discovered a bug. 'Show' always shows a single blank line, as if the top visible line were the top line of the file. One should not have to scroll to trigger the context calculation. This exists now, prior to the new PR. If/when we add a hotkey (M4), once should be able to show, see context, and hide just by hitting hotkey twice. Add C3: Make 'Show' show the context for the current top visible line.

    @taleinat
    Copy link
    Contributor

    taleinat commented Jul 17, 2019

    Terry, on latest master (with the aforementioned PR merged), the context is updated quickly after the code-context is shown. This happens after a delay of up to 100ms due to the update loop. #59026 for new C4.

    @terryjreedy
    Copy link
    Member Author

    New changeset e0a1f8f by Terry Jan Reedy (Tal Einat) in branch 'master':
    bpo-33610: IDLE's code-context always shows current context immediately (GH-14821)
    e0a1f8f

    @miss-islington
    Copy link
    Contributor

    New changeset db2957c by Miss Islington (bot) in branch '3.7':
    bpo-33610: IDLE's code-context always shows current context immediately (GH-14821)
    db2957c

    @terryjreedy
    Copy link
    Member Author

    New changeset 86eb5da by Terry Jan Reedy (Miss Islington (bot)) in branch '3.8':
    bpo-33610: IDLE's code-context always shows current context immediately (GH-14821) (bpo-14846)
    86eb5da

    @taleinat
    Copy link
    Contributor

    New changeset 1ebee37 by Tal Einat in branch 'master':
    bpo-33610: validate non-negative integer inputs in IDLE's config (GH-14822)
    1ebee37

    @miss-islington
    Copy link
    Contributor

    New changeset 5dab5e7 by Miss Islington (bot) in branch '3.8':
    bpo-33610: validate non-negative integer inputs in IDLE's config (GH-14822)
    5dab5e7

    @miss-islington
    Copy link
    Contributor

    New changeset 28815e0 by Miss Islington (bot) in branch '3.7':
    bpo-33610: validate non-negative integer inputs in IDLE's config (GH-14822)
    28815e0

    @terryjreedy terryjreedy added 3.10 only security fixes and removed 3.7 (EOL) end of life labels Dec 15, 2020
    @terryjreedy terryjreedy removed the 3.8 only security fixes label Dec 15, 2020
    @terryjreedy
    Copy link
    Member Author

    New changeset 6f79e60 by Terry Jan Reedy in branch 'master':
    bpo-33610: Edit idlelib.codecontext (GH-23773)
    6f79e60

    @miss-islington
    Copy link
    Contributor

    New changeset 925f998 by Miss Islington (bot) in branch '3.8':
    bpo-33610: Edit idlelib.codecontext (GH-23773)
    925f998

    @terryjreedy
    Copy link
    Member Author

    New changeset 99d37a0 by Miss Islington (bot) in branch '3.9':
    bpo-33610: Edit idlelib.codecontext (GH-23773) (GH-23775)
    99d37a0

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @erlend-aasland
    Copy link
    Contributor

    Are there more work to be done here, or can we close this issue?

    @python python deleted a comment from csabella Jul 27, 2022
    @python python deleted a comment from csabella Jul 27, 2022
    @python python deleted a comment from csabella Jul 27, 2022
    @python python deleted a comment from csabella Jul 27, 2022
    @python python deleted a comment from csabella Jul 28, 2022
    @python python deleted a comment from csabella Jul 28, 2022
    @python python deleted a comment from csabella Jul 28, 2022
    @python python deleted a comment from taleinat Jul 28, 2022
    @terryjreedy
    Copy link
    Member Author

    1 1/2 years after last activity, revised #77791 (comment) shows some undone tasks. These are copied to new issue #95352.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes topic-IDLE type-feature A feature request or enhancement
    Projects
    Status: Done
    Development

    No branches or pull requests

    5 participants