gh-42010: IDLE Editor Bottom Scroll Bar #1984

csabella · 2017-06-07T11:49:40Z

Adds a horizontal scroll bar to the IDLE Editor windows.

https://bugs.python.org/issue1207613

Issue: Idle Editor: Bottom Scroll Bar #42010

mlouielu · 2017-06-07T13:14:14Z

Lib/idlelib/editor.py

@@ -188,6 +189,9 @@ def __init__(self, flist=None, filename=None, key=None, root=None):
        vbar['command'] = text.yview
        vbar.pack(side=RIGHT, fill=Y)
        text['yscrollcommand'] = vbar.set
+        hbar['command'] = text.xview
+        hbar.pack(side='bottom', fill='x')


I think the fill='x' should change to fill=X.

@mlouielu 'X' or X ?

X, in tkinter.X

I don't think this request is going to be merged, or it will require many more changes before it is merged, so I'm going to hold off on additional commits for now.

Either 'x' or X where X == 'x'. For consistency, the latter, with X added to the import.

The more important change is that vbar is now gridded (hence conflict) and hbar must be also. textview.ScrollableTextFrame (wrap=None) uses sticky=EW.

terryjreedy

This patch was submitted when I was just getting started on github and before I was ready to make such a change. I am now. Summary of changes now:

Resolve the merge conflict. We now grid the vbar, and set the command to handle_yview, which scrolls by lines.
Use textview.AutoHideScrollbar (which requires gridding).
Disable hbar in PyShell, at least by default.
Test? We do not need to test the operation of AutoHideScrollbar. That should be in test.test_textview. But test that is present. Check font page font sample test.

Once working, we need to consider global and window configuration options.

terryjreedy · 2020-02-25T20:13:08Z

Lib/idlelib/editor.py

@@ -188,6 +189,9 @@ def __init__(self, flist=None, filename=None, key=None, root=None):
        vbar['command'] = text.yview
        vbar.pack(side=RIGHT, fill=Y)
        text['yscrollcommand'] = vbar.set
+        hbar['command'] = text.xview
+        hbar.pack(side='bottom', fill='x')


Either 'x' or X where X == 'x'. For consistency, the latter, with X added to the import.

terryjreedy · 2020-02-25T21:16:44Z

Lib/idlelib/editor.py

@@ -108,6 +108,7 @@ def __init__(self, flist=None, filename=None, key=None, root=None):
                'recent-files.lst')
        self.text_frame = text_frame = Frame(top)
        self.vbar = vbar = Scrollbar(text_frame, name='vbar')
+        self.hbar = hbar = Scrollbar(text_frame, orient="horizontal", name='hbar')


Use textview.AutoHideScrollbar. Unless there is a clear reason otherwise, set the command for this and vbar in the initial call. For the latter, use revised command=self.handle_yview. I don't believe we need to add a handle_xview.

terryjreedy · 2020-02-25T21:20:54Z

Lib/idlelib/editor.py

@@ -188,6 +189,9 @@ def __init__(self, flist=None, filename=None, key=None, root=None):
        vbar['command'] = text.yview
        vbar.pack(side=RIGHT, fill=Y)
        text['yscrollcommand'] = vbar.set
+        hbar['command'] = text.xview


As said above, set command when init and delete setting here.

terryjreedy · 2020-02-25T21:23:22Z

Lib/idlelib/editor.py

@@ -188,6 +189,9 @@ def __init__(self, flist=None, filename=None, key=None, root=None):
        vbar['command'] = text.yview
        vbar.pack(side=RIGHT, fill=Y)
        text['yscrollcommand'] = vbar.set
+        hbar['command'] = text.xview
+        hbar.pack(side='bottom', fill='x')


The more important change is that vbar is now gridded (hence conflict) and hbar must be also. textview.ScrollableTextFrame (wrap=None) uses sticky=EW.

bedevere-bot · 2020-02-25T21:33:40Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

terryjreedy · 2020-02-26T05:05:09Z

Rename AutoHideScrollbar to AutoShowScrollbar. It is hidden (ungridded) by default and normally, for many people, will almost never appear. With the better name, I might have realized that it does not need to be optional.

Cheryl, if you cannot revise soon (this week), please say so and I will do the above. 3.8.2 and 3.9.0a4 were released today, and I would like to merge well before the next releases.

csabella · 2020-02-26T12:38:29Z

I have made the requested changes; please review again.

Terry, I made the suggested changes, but I didn't get to look at it a lot. Bringing up editor.py in the 80-width window initially shows the horizontal scroll bar and it allows scrolling. Scrolling down to where the hbar isn't needed makes it disappear (hide) and then when it reappears, it's jumpy. The horizontal scroll wheel on mouse also doesn't work, but I'm not sure if it's even possible to add that functionality.

bedevere-bot · 2020-02-26T12:38:32Z

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

terryjreedy

Cheryl, I tested the patch and initially it seemed fine. Then I encountered the continued flashing that I presume you referred to, and discovered how to reproduce it, and figure out why, I think, but not how to stop it.

@taleinat We are trying to add the as-needed horizontal scrollbar used in the font sample to editor and output windows. When scrolling with the text height fixed, (the initial default), there are tolerable single 'flashes' when the bar is shown or hidden, like seen when context lines are added or deleted. When scrolling with the window height fixed, as when zoomed or after resizing, a minor glitch can happen when the bottom line is the only long line but is mostly covered up by the scrollbar, so that there is nothing visible to scroll. I will come back to how to reproduce this.

In either mode, a long line only at the bottom can cause the bbar to appear and disappear in a loop. To reproduce on my machine with this patch applied, using SourceCodePro size12 font, open configdialog.py. In fixed text mode, scroll down until blank line 252 is the bottom visible line. Move down 1 more with vbar bottom arrow. Long line 253 and hbar appear (with a flash). Move back up with top vbar arrow. Hbar flashes until one scrolls again.

Switch to fixed window mode by slightly increasing its height. Make non-blank line 251 the bottom visible line. A slice of blank line 252 is also displayed but with a small enough window height increase, none of line number 252 will be. Move down with vbar arrow, exposing blank line 252 and bit of long line 253, which will be covered by the hbar. Since none of 253 is visible, the hbar is not needed, so it is removed, exposing a slice of 253, making it needed again. and so on. (I presume something like this happens in the fixed text case when going down and up, even though the text window should be exactly n lines. Only the top (and blank) 1 pixel slice of the long line would be need to be exposed and covered.)

Now make 251 the maximum line number exposed and put the cursor there or above. Move the cursor down with the down arrow key, rather than clicking the vbar down arrow. When scrolling down with the cursor, lines are synchronized with the bottom of the text widget and partial lines are exposed at the top rather the bottom of the text widget. When the cursor exposes line 253, it will be covered up by the hbar. But for me, the top of the cursor is visible, meaning that the blank top slice of 253 is visible, hence the hbar is not removed. This is the minor glitch described above.

The question is how to stop the looping in the situations where it now occurs. I have some vague ideas, but need to sleep soon.

bedevere-bot · 2020-02-27T06:12:17Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

terryjreedy · 2020-02-27T06:25:28Z

I never heard of a horizontal scrollwheel. Must be a linux thing with different linux-only button numbers to be bound.

csabella · 2020-02-27T10:24:58Z

I never heard of a horizontal scrollwheel. Must be a linux thing with different linux-only button numbers to be bound.

Sorry I hadn't been more clear in my comment. It's actually a button on my mouse and it's not OS specific. I didn't look at the docs to see what it would map to (if anything) for Tck/Tk. It's just my first natural choice for left/right scrolling and I noticed it didn't work.

taleinat · 2020-02-27T11:49:26Z

I never heard of a horizontal scrollwheel.

They do exist, e.g. the Logitech MX Master mice have a second horizontal scroll-wheel.

terryjreedy · 2020-02-27T22:36:45Z

I was thinking of a 'wheel that scrolls horizontally' as being implemented either with a physical 2nd wheel or a shift button that makes the current wheel be interpreted horizontally (while held down).* Neither the left nor right button are suitable for that (they have other effects), but the editor currently ignores the middle (wheel) button. So we could use it for wheel mode shift. Holding the wheel down while rotating it is harder but is not too bad, especially after just a bit of practice.

I believe we could also make the wheel scroll vertically or horizontally when the mouse is over the corresponding scrollbar.

But we first need to solve the flash loop. I might ask on SO.

Whether those oddball mice trigger new tk events could be tested by binding mouse events in a blank Tk() to a handler that print()s events. I would not expect so. Buttons are limited to 1 to 5, so on Linux, using 6 & 7 for horizontal scrolling is not possible.

terryjreedy · 2020-02-28T03:41:41Z

One of my ideas was to 'de-bounce' hbar being displayed by rejecting following events that come too quickly. About 20 events per second should be about the fastest for auto-repeat keys and wheel click. I need to see how fast the scrollbar is responding to it hiding the long line that made it show.

taleinat · 2020-05-20T16:30:01Z

I've worked on this on and off, and all I can say is that the Tk text widget is pretty bad with regard to horizontal scrolling: It sets the horizontal scroll position only according to the currently visible lines! If the text was scrolled to the right, scrolling down to where there are only shorter lines will scroll the text back to the left!! 😠

I've not found any way to change this or work around it, having spent too much time trying... I think the best we can do without going overboard with this is to be make sure to keep the horizontal scrollbar displayed whenever the contents of the text widget are wider than it's display area.

My suggestion is to have a separate scrollbar class ("AutoShowHorizontalScollbarForText" ?), identical to the existing AutoShowScrollbar but with this for its set() method:

def set(self, lo, hi):
    super().set(lo, hi)
    text = self.text_widget
    lines = text.get('1.0', 'end-1c').split('\n')
    longest_line = max(lines, key=len)
    width = Font(text, text['font']).measure(longest_line)
    inner_padding = sum(map(text.tk.getint, [text.cget('border'),
                                             text.cget('padx')]))
    show = width > text.winfo_width() - 2 * inner_padding
    if show:
        self.grid()
    else:
        self.grid_remove()

In my testing, this completely avoids the issues described in previous comments and does not significantly affect the performance of scrolling. I think this is a relatively simple solution that is a clear improvement on the status quo.

taleinat · 2020-05-20T16:38:29Z

The other possibility I can think of (the aforementioned "going overboard") is to put the text widget in a frame which auto-resizes to fit the text widget, and then have the outer frame scrolled instead of the text widget, writing the adapters to make it all work. This includes special handling not only of scrolling logic, but also more esoteric text methods such as Text.see().

taleinat · 2020-09-20T17:59:01Z

I spent hours trying to get this to work with the Text widget - I couldn't find any way to get it working without jumping around when scrolling vertically due to different line lengths and Tk's Text widget's strange behavior regarding them.

As it is, IMO this PR should be closed.

If someone would like to tackle this, a different approach would be needed than the one used here.

taleinat · 2020-09-20T18:39:54Z

@terryjreedy, I propose closing this PR.

terryjreedy · 2020-11-02T03:52:35Z

Were the hours spent with the replacement 'see' method or the 'overboard' or both?

I would be willing to make this an option if the partial line glitch were fixed (by never allowing them) and the looping were fixed (by adding a much longer delay between changes?).

taleinat · 2020-11-02T07:45:23Z

Were the hours spent with the replacement 'see' method or the 'overboard' or both?

The hours were spent trying various ways to get Tk to scroll the text widget properly, reading through its docs, searching online etc. I couldn't find any way to avoid it jumping back left when scrolling up/down to an area with shorter lines.

The alternatives would be effectively implementing scrolling behavior ourselves, but I can't possibly see that being worth the extra effort, maintenance burden and potential performance hit for large files.

I would be willing to make this an option if the partial line glitch were fixed (by never allowing them) and the looping were fixed (by adding a much longer delay between changes?).

I couldn't find any reasonable way to achieve these, hence my recommendation to close this.

github-actions · 2022-02-19T00:08:50Z

This PR is stale because it has been open for 30 days with no activity.

ambv · 2022-05-17T16:47:48Z

This missed the boat for inclusion in Python 3.9 which accepts security fixes only as of today.

github-actions · 2022-06-18T00:11:29Z

This PR is stale because it has been open for 30 days with no activity.

erlend-aasland · 2022-06-29T19:28:56Z

Quoting @taleinat's latest post here:

I couldn't find any reasonable way to achieve these, hence my recommendation to close this.

Suggesting to apply the pending close label on this PR, unless there still is interest to drive this forward. cc. @terryjreedy @csabella

terryjreedy · 2022-07-01T03:20:40Z

I am leaving the issue open and added a comment there.

the-knights-who-say-ni added the CLA signed label Jun 7, 2017

mlouielu reviewed Jun 7, 2017

View reviewed changes

brettcannon added the awaiting review label Feb 2, 2018

terryjreedy self-assigned this Feb 25, 2020

terryjreedy requested changes Feb 25, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Feb 25, 2020

terryjreedy added DO-NOT-MERGE needs backport to 3.7 labels Feb 25, 2020

bpo-1207613: IDLE Editor Bottom Scroll Bar

90ba632

csabella force-pushed the horz_scroll branch from 2053b25 to 90ba632 Compare February 26, 2020 11:54

Update to AutoShowScrollbar

568ac0f

bedevere-bot added awaiting change review and removed awaiting changes labels Feb 26, 2020

bedevere-bot requested a review from terryjreedy February 26, 2020 12:38

terryjreedy requested a review from taleinat February 27, 2020 05:06

terryjreedy requested changes Feb 27, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Feb 27, 2020

python deleted a comment from codecov bot Mar 31, 2020

taleinat added the needs backport to 3.9 only security fixes label May 22, 2020

ned-deily removed the needs backport to 3.7 label Jun 28, 2020

ambv removed the needs backport to 3.8 label May 4, 2021

github-actions bot added the stale Stale PR or inactive for long period of time. label Feb 19, 2022

terryjreedy mentioned this pull request Mar 25, 2022

bpo-1207613: Add scrollbar to IDLE editor #32116

Closed

ambv removed the needs backport to 3.9 only security fixes label May 17, 2022

github-actions bot removed the stale Stale PR or inactive for long period of time. label May 18, 2022

github-actions bot added the stale Stale PR or inactive for long period of time. label Jun 18, 2022

erlend-aasland changed the title ~~bpo-1207613: IDLE Editor Bottom Scroll Bar~~ gh-42010: IDLE Editor Bottom Scroll Bar Jun 29, 2022

github-actions bot removed the stale Stale PR or inactive for long period of time. label Jun 30, 2022

terryjreedy closed this Jul 1, 2022

voidspace mentioned this pull request Jul 1, 2022

Idle Editor: Bottom Scroll Bar #42010

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-42010: IDLE Editor Bottom Scroll Bar #1984

gh-42010: IDLE Editor Bottom Scroll Bar #1984

csabella commented Jun 7, 2017 •

edited by bedevere-bot

Loading

mlouielu Jun 7, 2017

matrixise Jun 9, 2017

mlouielu Jun 14, 2017

csabella Jun 14, 2017

terryjreedy Feb 25, 2020

terryjreedy Feb 25, 2020

terryjreedy left a comment

terryjreedy Feb 25, 2020

terryjreedy Feb 25, 2020

terryjreedy Feb 25, 2020

terryjreedy Feb 25, 2020

bedevere-bot commented Feb 25, 2020

terryjreedy commented Feb 26, 2020

csabella commented Feb 26, 2020

bedevere-bot commented Feb 26, 2020

terryjreedy left a comment

bedevere-bot commented Feb 27, 2020

terryjreedy commented Feb 27, 2020

csabella commented Feb 27, 2020

taleinat commented Feb 27, 2020

terryjreedy commented Feb 27, 2020

terryjreedy commented Feb 28, 2020

taleinat commented May 20, 2020 •

edited

Loading

taleinat commented May 20, 2020 •

edited

Loading

taleinat commented Sep 20, 2020

taleinat commented Sep 20, 2020

terryjreedy commented Nov 2, 2020 •

edited

Loading

taleinat commented Nov 2, 2020

github-actions bot commented Feb 19, 2022

ambv commented May 17, 2022

github-actions bot commented Jun 18, 2022

erlend-aasland commented Jun 29, 2022

terryjreedy commented Jul 1, 2022

gh-42010: IDLE Editor Bottom Scroll Bar #1984

gh-42010: IDLE Editor Bottom Scroll Bar #1984

Conversation

csabella commented Jun 7, 2017 • edited by bedevere-bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terryjreedy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bedevere-bot commented Feb 25, 2020

terryjreedy commented Feb 26, 2020

csabella commented Feb 26, 2020

bedevere-bot commented Feb 26, 2020

terryjreedy left a comment

Choose a reason for hiding this comment

bedevere-bot commented Feb 27, 2020

terryjreedy commented Feb 27, 2020

csabella commented Feb 27, 2020

taleinat commented Feb 27, 2020

terryjreedy commented Feb 27, 2020

terryjreedy commented Feb 28, 2020

taleinat commented May 20, 2020 • edited Loading

taleinat commented May 20, 2020 • edited Loading

taleinat commented Sep 20, 2020

taleinat commented Sep 20, 2020

terryjreedy commented Nov 2, 2020 • edited Loading

taleinat commented Nov 2, 2020

github-actions bot commented Feb 19, 2022

ambv commented May 17, 2022

github-actions bot commented Jun 18, 2022

erlend-aasland commented Jun 29, 2022

terryjreedy commented Jul 1, 2022

csabella commented Jun 7, 2017 •

edited by bedevere-bot

Loading

taleinat commented May 20, 2020 •

edited

Loading

taleinat commented May 20, 2020 •

edited

Loading

terryjreedy commented Nov 2, 2020 •

edited

Loading