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

Squeezer - squeeze large output in IDLE's shell #43728

Closed
taleinat opened this issue Jul 27, 2006 · 59 comments
Closed

Squeezer - squeeze large output in IDLE's shell #43728

taleinat opened this issue Jul 27, 2006 · 59 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 (EOL) end of life topic-IDLE type-feature A feature request or enhancement

Comments

@taleinat
Copy link
Contributor

BPO 1529353
Nosy @terryjreedy, @taleinat, @serhiy-storchaka, @csabella, @miss-islington
PRs
  • bpo-1529353: IDLE: squeeze large output in the shell #7626
  • [3.7] bpo-1529353: IDLE: squeeze large output in the shell (GH-7626) #9561
  • [3.6] bpo-1529353: IDLE: squeeze large output in the shell (GH-7626) #9562
  • bpo-1529353: IDLE: Squeezer What's New for 3.6.7 #9567
  • bpo-1529353: IDLE - Squeezer What's New for 3.7.1 #9568
  • [3.7] bpo-1529353: IDLE: Squeezer What's New for 3.6.7 (GH-9567) #9569
  • [3.6] bpo-1529353: IDLE: Squeezer What's New for 3.6.7 (GH-9567) #9570
  • [3.7] bpo-1529353: IDLE - Squeezer What's New for 3.7.1 (GH-9568) #9571
  • bpo-1529353: IDLE - Squeezer What's New for 3.8 #9572
  • bpo-1529353: Explain Shell text squeezing in the IDLE doc. #10169
  • [3.7] bpo-1529353: Explain Shell text squeezing in the IDLE doc. (GH-10169) #10179
  • [3.6] bpo-1529353: Explain Shell text squeezing in the IDLE doc. (GH-10169) #10180
  • Dependencies
  • bpo-33839: IDLE tooltips.py: refactor and add docstrings and tests
  • Files
  • Squeezer.py: Noam Raphael's original Squeezer.py (from IDLE-fork)
  • IDLE_Squeezer.patch: initial patch
  • taleinat.20140211.IDLE_Squeezer.patch: modern patch for Python 3.4
  • Squeezer_Screenshot.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 2018-09-26.11:40:17.976>
    created_at = <Date 2006-07-27.00:13:56.000>
    labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7']
    title = "Squeezer - squeeze large output in IDLE's shell"
    updated_at = <Date 2018-10-28.17:08:26.340>
    user = 'https://github.com/taleinat'

    bugs.python.org fields:

    activity = <Date 2018-10-28.17:08:26.340>
    actor = 'miss-islington'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2018-09-26.11:40:17.976>
    closer = 'terry.reedy'
    components = ['IDLE']
    creation = <Date 2006-07-27.00:13:56.000>
    creator = 'taleinat'
    dependencies = ['33839']
    files = ['7428', '7429', '34050', '34209']
    hgrepos = []
    issue_num = 1529353
    keywords = ['patch']
    message_count = 59.0
    messages = ['50765', '50766', '50767', '50768', '50769', '84637', '84645', '84657', '210296', '210310', '211027', '211068', '211071', '211075', '211076', '211083', '211643', '211645', '211646', '211647', '212021', '212061', '319291', '319316', '319317', '319319', '319339', '319344', '319353', '319367', '319428', '319430', '319452', '319494', '319495', '319496', '323471', '324255', '324365', '324374', '326100', '326310', '326343', '326347', '326348', '326349', '326350', '326368', '326369', '326370', '326371', '326372', '326373', '326374', '326419', '326445', '328703', '328710', '328711']
    nosy_count = 5.0
    nosy_names = ['terry.reedy', 'taleinat', 'serhiy.storchaka', 'cheryl.sabella', 'miss-islington']
    pr_nums = ['7626', '9561', '9562', '9567', '9568', '9569', '9570', '9571', '9572', '10169', '10179', '10180']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue1529353'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

    @taleinat
    Copy link
    Contributor Author

    Here is my improved version of Squeezer - the IDLE
    extension to end all of your
    oh-no-I-just-printed-War-and-Peace-in-the-interpreter
    troubles!

    Changes:

    • Fixed line counting logic
    • Line counting is done according to current window
      width and current tab width
    • Fixed configHandler to enable reading of raw data
      from config files (100% backwards compatible) - needed
      by Squeezer to enable configuration of text viewers in
      config-extensions

    I've been working this vesion without a hitch for
    several weeks, but that's hardly enough testing is it?

    Attached:

    • the original Squeezer.py by Noam Raphael
    • a diff against Noam's original Squeezer.py
    • a diff of configHandler.py against the current SVN
      version (revision 46863)
    • fixed 'middle click to copy' - now works on Windows

    One minor note - the 'middle click to copy' didn't work
    on Windows because it simply called selection_own()
    which doesn't affect the clipboard. I added
    clipboard_clear() and clipboard_append() and it now
    works on Windows. This should be tested on Linux, Unix,
    OSX etc. to check compatibility.

    For more info on the clipboard issue, see:
    http://groups.google.com/group/comp.lang.python/browse_thread/thread/647feeef55625733/bc08421d8aea9928?lnk=st&q=&rnum=2#bc08421d8aea9928

    @kbkaiser
    Copy link
    Contributor

    Logged In: YES
    user_id=149084

    OK, thanks. Please take a look at the two diffs you
    uploaded, they seem corrupted. Squeezer.py looks ok.

    @taleinat
    Copy link
    Contributor Author

    Logged In: YES
    user_id=1330769

    Uploaded unified diffs instead of standard diffs. Probably
    the first diffs had bad newlines because I'm working on Win.

    Also 2 changes:

    • minor fix - end case when clicking at beginning of text block
    • Allow squeezing of stderr (tracebacks...)

    @kbkaiser
    Copy link
    Contributor

    Logged In: YES
    user_id=149084

    Ah, that's better. Thanks.

    @kbkaiser
    Copy link
    Contributor

    kbkaiser commented Feb 5, 2007

    Delete the patch to configHandler.py, implemented with Patch 1650174.

    @devdanzin devdanzin mannequin added type-feature A feature request or enhancement labels Mar 30, 2009
    @taleinat
    Copy link
    Contributor Author

    "Test needed"? I'll need a bit of guidance on this.

    Has there been a change of policy of which I'm not aware, that patches
    to IDLE not going to be accepted unless comprehensive tests are included?

    Much of IDLE doesn't include tests, e.g. the RPC code. There's a comment
    by K.B.K. in the end of rpc.py from September '03 saying we need a
    proper test suite for it, and testing that should be simple compared to
    testing GUI related functionality.

    In any case, if someone can suggest a simple approach to test Squeezer
    I'll gladly work it up. As it is, I can think of ways to test certain
    functions, but I'm not sure how to approach testing GUI related
    functionality (or if I should approach this at all).

    @devdanzin
    Copy link
    Mannequin

    devdanzin mannequin commented Mar 30, 2009

    Tal,
    There is no such policy AFAIK. The stage field is a tool to help us
    handle the issues, sorry to give you a wrong impression. If IDLE has no
    tests, 'test needed' doesn't apply here, so we can set this to 'patch
    review'.

    If someone is already working on adding tests for IDLE, please revert
    this to 'test needed'.

    @taleinat
    Copy link
    Contributor Author

    Daniel, Thanks for clearing that up :)

    @taleinat
    Copy link
    Contributor Author

    taleinat commented Feb 5, 2014

    Ping?

    Can this please go in? It's such a great improvement to user experience!

    I've seen too many novice users print a ton of output by accident and have IDLE hang as a result, losing their interactive session. This simple extension avoid those issues completely, while still allowing access to the output if needed.

    I'd even be willing to do any extra work required, such as testing on Windows, Linux and OSX. Just tell me what's holding this up!

    @serhiy-storchaka
    Copy link
    Member

    Could you please provide a patch which applies to default branch?

    There is other proposed solution for the problem of large output: bpo-1442493.

    @taleinat
    Copy link
    Contributor Author

    Here's a completely re-written patch!

    The patch is against current 3.3 (changeset 360976a6d8b9), since that's what Terry requested recently on bpo-3068. It also applies cleanly to the current default branch (changeset 360976a6d8b9).

    This patch has cleaner, modern code, with doc-strings everywhere and informative comments where appropriate. It also uses "import tkinter as tk" as Terry recently requested on bpo-3068.

    Furthermore, I've added a very complete set of tests for the new extension, which tests everything I could think of and achieves 100% coverage (!) of Squeezer.py. These are modern tests which make extensive use of mocking via the awesome unittest.mock module. I've included extensive in-code comments with the tests. Hopefully these will be useful as a reference for writing additional tests for IDLE code (at least other extensions).

    Finally, this patch includes a minor change to ToolTip.py, adding the ability to specify the delay after which a tooltip is displayed (was hard-coded to 1.5 seconds). This change is required by the extension (I prefer that it uses tooltips with no delay).

    @serhiy-storchaka
    Copy link
    Member

    Expanded text still makes IDLE unresponsible. Try print('a'*10000000).

    I think it will be better to output first 100 lines and a button "More", pressing on which expands next 100 lines.

    @serhiy-storchaka
    Copy link
    Member

    During experiments I got following message on terminal:

    Exception in Tkinter callback
    Traceback (most recent call last):
      File "/home/serhiy/py/cpython/Lib/tkinter/__init__.py", line 1482, in __call__
        return self.func(*args)
      File "/home/serhiy/py/cpython/Lib/tkinter/__init__.py", line 534, in callit
        func(*args)
      File "/home/serhiy/py/cpython/Lib/idlelib/ToolTip.py", line 44, in showtip
        x = self.button.winfo_rootx() + 20
      File "/home/serhiy/py/cpython/Lib/tkinter/__init__.py", line 845, in winfo_rootx
        self.tk.call('winfo', 'rootx', self._w))
    _tkinter.TclError: bad window path name ".3066068076.3066068332.text.3048812460"

    May be it is unrelated.

    @taleinat
    Copy link
    Contributor Author

    Regarding the tooltip traceback, seems like that could be related. I added a tooltip to the "squeezed text" buttons, with no delay, so perhaps there's some kind of race condition?

    Could you try changing "delay=0" in the code to "delay=50" and see if the issue goes away?

    I would test this myself if you could specify how to reproduce that error.

    @serhiy-storchaka
    Copy link
    Member

    I'm not able to reproduce the problem after the first time (even with
    "delay=0").

    @taleinat
    Copy link
    Contributor Author

    Indeed, expanding very long texts still degrades the performance of IDLE. But the user has to explicitly expand the text (by double-clicking the button) and can easily squeeze the text again.

    As for your suggestion to show just the first several lines and a "more..." button, that would be a significantly more complex solution. It is already easy to preview the squeezed text, either by launching an external viewer or by copying the text to the clipboard.

    Yes, a user could still get large amount of text in the shell by expanding large squeezed text. In other words, a user can still shoot himself in the foot; but now he'd have to purposefully aim the shotgun at his foot first.

    @serhiy-storchaka
    Copy link
    Member

    The test emits a lot of warnings.

    @taleinat
    Copy link
    Contributor Author

    Serhiy, which warnings?

    Some of the tests use Tk GUI elements, so they include "requires('gui')". You need to run the tests with a proper environment so that these tests are run; otherwise you get a warning that the 'gui' resource in unavailable.

    @taleinat
    Copy link
    Contributor Author

    See related bpo-6143 "IDLE - an extension to clear the shell window", where special support for Squeezer is required in that extension (patch included there).

    @serhiy-storchaka
    Copy link
    Member

    Mainly deprecation warnings about assertEquals(). Run

    ./python -Wall -b -m test.regrtest -vuall test_idle
    

    @taleinat
    Copy link
    Contributor Author

    See msg212020 for a Python-style explanation of the utility of the Squeezer extension.

    @taleinat
    Copy link
    Contributor Author

    Added screenshot of what working with the IDLE shell with the Squeezer extension looks like (on Windows).

    Note that the second squeezed text is the very long "Max recursion depth" exception traceback, which I manually squeezed after it was printed.

    @taleinat
    Copy link
    Contributor Author

    New PR ready for review.

    I've updated the code for recent master and cleaned up the deprecation warnings from the tests.

    @taleinat taleinat added the 3.7 (EOL) end of life label Jun 11, 2018
    @taleinat
    Copy link
    Contributor Author

    I'm back to working on this. I've made good progress: I'm nearly done with all of the changes suggested here and ready for another PR review.

    I'd like some opinions on a few things:

    1. Should the tooltip-related configuration options be removed? There is one for whether to show tooltips for squeezed outputs and another to set the hover delay. I feel that they add unnecessary complexity.

    2. Should there be an option to entirely disable auto-squeezing? Currently there is only a setting for the minimum number of lines, which can just be set very high.

    3. Should we keep the events for squeezing the last output block and expanding the last squeezed output block? ISTM the keyboard shortcuts would only be used by "power users", which aren't our main target. Further, without keyboard shortcuts, I see no point for having these events at all, given that it is easy to achieve this otherwise.

    4. Is it worth the effort to update the number of lines displayed for squeezed outputs when a shell window's width is changed?

    @pppery pppery mannequin changed the title Squeezer - squeeze large output in the interpreter Squeezer - squeeze large output in IDLE Aug 15, 2018
    @pppery pppery mannequin changed the title Squeezer - squeeze large output in the interpreter Squeezer - squeeze large output in IDLE Aug 15, 2018
    @taleinat taleinat changed the title Squeezer - squeeze large output in IDLE Squeezer - squeeze large output in IDLE's shell Aug 16, 2018
    @taleinat taleinat changed the title Squeezer - squeeze large output in IDLE Squeezer - squeeze large output in IDLE's shell Aug 16, 2018
    @taleinat
    Copy link
    Contributor Author

    I've gone ahead and addressed all of the significant issues raised here:

    • Squeezer is no longer an extension
    • Expanding problematically large outputs prompts the user for confirmation
    • "copy" and "view" moved to a right-click context menu
    • use IDLE's TextView for viewing squeezed outputs
      • I added the ability to set the text wrapping
    • default minimum # lines for auto-squeezing increased to 50

    I've also updated all of the tests accordingly and added a few.

    There a several tweaks to consider, including those mentioned in my previous message, plus colors and key bindings.

    This is now ready for another review!

    @terryjreedy
    Copy link
    Member

    I partly answered questions already.

    1. Yes. Always show. Fix delay at 80 until we decide on something better.

    2. No. Max should be enough. I once printed over 500_000 short lines to see if scrolling remained responsive. It did. I could have set min to 1_000_000 for that experiment. Am I correct in presuming that only one block of output, between code imputs, can be squeezed?

    Converting extensions to features mostly removed the option to omit features.

    1. No, if not gone already. I don't want to proliferate keyboard shortcuts, at least not until we get rid of some that must be nearly unused.

    I want to make revising the use of keys within output blocks a separate issue.

    1. Maybe nnnn chars would be better.

    'a'*1000 currently wraps to 13 80-char lines. (This would change if we add horizontal scrollbar, as has been requested.) The viewer does not wrap. I think it should as there is now no way to see entire line. Or it needs a scrollbar. Viewer is modal, but does it need to be?

    @taleinat
    Copy link
    Contributor Author

    1. Yes. Always show. Fix delay at 80 until we decide on something better.

    Done.

    1. No. Max should be enough.

    Done.

    I once printed over 500_000 short lines to see if scrolling remained responsive. It did. I could have set min to 1_000_000 for that experiment. Am I correct in presuming that only one block of output, between code imputs, can be squeezed?

    Indeed.

    1. No, if not gone already. I don't want to proliferate keyboard shortcuts, at least not until we get rid of some that must be nearly unused.

    Done.

    1. Maybe nnnn chars would be better.

    I'm still not sure, that just leads to very large numbers, where it's hard to judge what is excessive, e.g., "is 100,000 chars too much"? With lines, I feel it is more obvious: "1,000 lines? That's way too much!"

    The viewer does not wrap. I think it should as there is now no way to see entire line. Or it needs a scrollbar.

    Wrapping is the major cause of the text widget slowing down, which is why I've made the viewer support controlling the wrapping mode, and made Squeezer use no wrapping. I've now added a horizontal scrollbar, and also made the scrollbars in the viewer appear only when needed. Now, scrolling horizontally with very long lines is still slow, but at least just the viewer is affected.

    Viewer is modal, but does it need to be?

    No, good catch, changed to non-modal.

    @taleinat
    Copy link
    Contributor Author

    The PR is ready for another review.

    ISTM it should be good to go, after have addressing all of the significant issues brought up in the discussion here.

    @terryjreedy
    Copy link
    Member

    Tal, if you think this is ready, I would like to get this in the upcoming releases. I believe the deadline is midnight at the international dateline, which is noon UTC tomorrow, just over 9 hours from now.

    I decided that any suggestions I might have for the non-user-visible aspects of squeezer.py and test_squeezer.py can wait for later. The best review of user-visible aspects will come from using it.

    A second use of squeezer should be debugger's global and non-global('local') namespace displays. The intended format is one name-value pair per line. But values can be indefinitely large. For this, we could skip the context menu (there is not one now), use a fixed trigger of n characters, and have double-click display to a popup Squeezer View.

    I will come back to this in a couple of hours to see if you have responded. I would write and merge a separate expanded news entry for idlelib/NEWS.txt.

    @taleinat
    Copy link
    Contributor Author

    Working on getting this in now. Just in time, it seems.

    @taleinat
    Copy link
    Contributor Author

    This seems like it would need a "What's New" entry.

    How does this work with IDLE re. new features being backported?

    @taleinat
    Copy link
    Contributor Author

    New changeset 604e7b9 by Tal Einat in branch 'master':
    bpo-1529353: IDLE: squeeze large output in the shell (GH-7626)
    604e7b9

    @miss-islington
    Copy link
    Contributor

    New changeset 321f28c by Miss Islington (bot) in branch '3.7':
    bpo-1529353: IDLE: squeeze large output in the shell (GH-7626)
    321f28c

    @miss-islington
    Copy link
    Contributor

    New changeset 0b3e120 by Miss Islington (bot) in branch '3.6':
    bpo-1529353: IDLE: squeeze large output in the shell (GH-7626)
    0b3e120

    @terryjreedy
    Copy link
    Member

    New changeset dac712d by Terry Jan Reedy in branch 'master':
    bpo-1529353: IDLE: Squeezer What's New for 3.6.7 (bpo-9567)
    dac712d

    @terryjreedy
    Copy link
    Member

    New changeset ea718d3 by Terry Jan Reedy in branch 'master':
    bpo-1529353: IDLE - Squeezer What's New for 3.7.1 (bpo-9568)
    ea718d3

    @miss-islington
    Copy link
    Contributor

    New changeset 98c8236 by Miss Islington (bot) in branch '3.6':
    bpo-1529353: IDLE: Squeezer What's New for 3.6.7 (GH-9567)
    98c8236

    @miss-islington
    Copy link
    Contributor

    New changeset 92ad261 by Miss Islington (bot) in branch '3.7':
    bpo-1529353: IDLE: Squeezer What's New for 3.6.7 (GH-9567)
    92ad261

    @miss-islington
    Copy link
    Contributor

    New changeset 3637e68 by Miss Islington (bot) in branch '3.7':
    bpo-1529353: IDLE - Squeezer What's New for 3.7.1 (GH-9568)
    3637e68

    @terryjreedy
    Copy link
    Member

    See my 3.6/3.7 patches for how I now handle new-in-maintenance-release user-visible features in What's New. Note that there are similar entries for Python at the bottom of the file.

    If I were not rushing, I would have mentioned user colors in text view. But that is minor and I will add one entry for colors and font sizes in both text and help views when done.

    @terryjreedy
    Copy link
    Member

    New changeset fdcb5ae by Terry Jan Reedy in branch 'master':
    bpo-1529353: IDLE - Squeezer What's New for 3.8 (bpo-9572)
    fdcb5ae

    @taleinat
    Copy link
    Contributor Author

    ISTM that this issue can finally be closed!

    Followup on some remaining minor points brought up here, such as colors and squeezing of multiple output lines printed separately, could be done in separate issues or discussed on idle-dev.

    @terryjreedy
    Copy link
    Member

    Yes. Congratulations on your persistence.

    @terryjreedy
    Copy link
    Member

    New changeset 68d6dc0 by Terry Jan Reedy in branch 'master':
    bpo-1529353: Explain Shell text squeezing in the IDLE doc. (bpo-10169)
    68d6dc0

    @miss-islington
    Copy link
    Contributor

    New changeset 8dccb00 by Miss Islington (bot) in branch '3.7':
    bpo-1529353: Explain Shell text squeezing in the IDLE doc. (GH-10169)
    8dccb00

    @miss-islington
    Copy link
    Contributor

    New changeset 69ab28d by Miss Islington (bot) in branch '3.6':
    bpo-1529353: Explain Shell text squeezing in the IDLE doc. (GH-10169)
    69ab28d

    @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 (EOL) end of life 3.8 (EOL) end of life topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants