Skip to content

IDLE: Path Browser vertical spacing is too small, causing the text to be cut off #122392

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

Closed
Wulian233 opened this issue Jul 29, 2024 · 16 comments
Closed
Labels
topic-IDLE type-bug An unexpected behavior, bug, or error

Comments

@Wulian233
Copy link
Contributor

Wulian233 commented Jul 29, 2024

Bug description:

Noise: @terryjreedy

The vertical spacing is too small, causing the text to be cut off. I'm not sure if this issue exists on other systems as well. Currently, I may not create a PR to fix it because I haven't found where the related code is and what the cause of the bug is. IDLE is a program with a long history; the folder icons here are very "retro"old, and the code is quite heavy.

2024-07-29 193916

Operating systems tested on:

Windows or all

Linked PRs

@Wulian233 Wulian233 added the type-bug An unexpected behavior, bug, or error label Jul 29, 2024
@terryjreedy
Copy link
Member

There is a GH project page for IDLE issues. Once an issue to added to the project on the right sidebar, one can click to open it. By default, IDLE issues is sorted by Topic. These mostly refer a block of menu entries. For instance, File - New File to File - Path Browser constitute the topic File open. There are 4 previous open browser issues. The main one is #75733, converting the browsers to be based on ttk.Treeview instead of Canvas, as currently in tree.py. At least some of the enhancements listed at the top of tree.py should be part of or easier with Treeview.

I temporarily fixed browser line spacing a decade ago in #66818 by increasing tree.py line 202 (first after def draw) to dy = 20. The issue reappeared 5 years ago in #81222 on hi-res monitors. Is this your case? I closed #81222 in favor of #75733, intending to fix line spacing as part of the conversion. #81222 has some Treeview test code, though dynamically setting spacing to the monitor seems not trivial.

So browser line spacing is a duplicate issue. However, since using Treeview is still in the future, I will consider patching tree.py to calculate the line spacing. Such code might be reusable with Treeview. Currently the browsers use the same system font as used for the menus, which is not changed by user font selections. As long as this is true, spacing could be calculated on startup rather than just for each browser. My current idea is to write a line, say, 'sys.path' (with descending chars) in a text widget and get the height of the bounding box. I will do some experiments next.

I would need help with code and review for these issues. How familiar are you with tkinter? Any experience with ttk styles?

Removing unneeded icons is #69277. That will definitely be easier with Treeview. The browser icons are folder.git, minusnode.gif, openfolder.gif, and plusnode.gif in idlelib.Icons. They fit within 16x16 pixels. Such files could be replaced if someone 'paints' new ones from scratch, without directly copying from elsewhere, and contributes them. Do you have anything in mind? Adding new icons, possibly blank for files and functions might be an alternative to simply removing the python icon.

@terryjreedy
Copy link
Member

Please copy the following into a file and run it.

from tkinter import *
r = Tk()
r.lift()
t = Text(r)
t.pack()
t.insert('1.0','Ãny')
r.update_idletasks()
r.lift()
print(t.bbox('1.0'), t.bbox('1.1'), t.bbox('1.2'))
def Ãnyf(): pass
class Ãnyc(): pass

I see (2, 2, 8, 16) (10, 2, 8, 16) (18, 2, 8, 16). Although the font in the tk window is not the same as the module browser window, the height of Ãny is the same as far as I can tell, and the spacing of 16 + 4 = 20 pixels is more than adequate. Change dy in your tree.py to the reported height plus a bit and then try module browser on this or other .py file. I going to repeat with text written on a canvas.

@Wulian233
Copy link
Contributor Author

Wulian233 commented Jul 30, 2024

I see (2, 2, 7, 13) (9, 2, 7, 13) (16, 2, 7, 13). dy=25 is fine for me
2024-07-30 082932

I would need help with code and review for these issues. How familiar are you with tkinter? Any experience with ttk styles?

I've used ttk to create some simple utility GUIs before. They were quite simple, so I didn't use many components. One example (ignore the non-English parts) is this GitHub project. Although the appearance differs from the default tkinter style, that's because I used the sv-ttk modulehttps://github.com/rdbende/Sun-Valley-ttk-theme for styling, which is just a ttk/tcl theme. It's entirely written with ttk

I'm very willing to help!
2024-07-30 084308

I have a radical idea: path browser is create a new window to preview folders by clicking to view subfolders or files, which is basically the same as the file Explorer that comes with the system. We can use os.startfile(sys.path) to open the folder and delete tree.py (os.startfile Windows only, We need to be compatible with different systems)

import os
import sys
import subprocess

def path_browser(path):
    system = sys.platform()
    if system == "win32":
        os.startfile(path)
    elif system == "darwin":
        subprocess.Popen(["open", path])
    elif system == "linux":
        subprocess.Popen(["xdg-open", path])
    else:
        raise OSError("Unsupported operating system")


folder_path = "/path/to/your/folder"
path_browser(folder_path)

Other places that use the path browser also use path_browser func instead. like https://github.com/python/cpython/blob/main/Lib/idlelib/editor.py#L785-L788

@terryjreedy
Copy link
Member

terryjreedy commented Jul 30, 2024

Since 13 is shorter than 16, it puzzles me that you need a greater height to avoid overlap. I did a similar test as above with canvas.create_text and got a height of 17. But this is only used for extra label text in stackviewer. The browsers display object text as labels within windows on the canvas. I believe this is to get event binding (but I believe this could be done with canvas texts). Try this code.

from tkinter import *
r = Tk()
r.lift()
c = Canvas(r)
c.pack()
l = Label(c, text='Ãny', bd=0, padx=2, pady=2)
l2 = Label(c, text='Ãny', bd=0, padx=2, pady=2)
w = c.create_window(2, 2, anchor=NW, window=l)
c.create_window(2, 22, anchor=NW, window=l2)  # One widget can only be in one window.
r.update_idletasks()
r.lift()
print(c.bbox(w))

I get (2, 2, 27, 21). I should really have dy = 21 = 17 + 4 padding, set to work on Windows. The 1 pixel overlap is visible with careful observation of selection highlighting. What is your OS, what is printed, and do you see overlap with this code with dy=20?

@Wulian233
Copy link
Contributor Author

Wulian233 commented Jul 30, 2024

I get (2, 2, 27, 23). Win10 22H2 laptop: 1920x1080 150% Dpi

20

20

21

21

25

25

@terryjreedy
Copy link
Member

This is making more sense. How about with 23?

@Wulian233
Copy link
Contributor Author

23

23

@terryjreedy
Copy link
Member

Thanks. The bounding box height looks like it will work. I am thinking of the best way to get it.

@Xiaokang2022
Copy link
Contributor

Xiaokang2022 commented Oct 4, 2024

Based on the above discussion, after I looked at the source code and found that we can set the size of dy by calculating the height of the first row.

This value is not used in the first row, so we can set its initial value to 0 for later judgment. After the first row is drawn, taking its height and assigning it to dy will solve the problem.

I've created a PR to fix this, it should help.

@Xiaokang2022
Copy link
Contributor

Xiaokang2022 commented Oct 4, 2024

In the PR, I tested it with my Win10 22H2 laptop (1920x1080 150%), here I tested it with my Win11 23H2 laptop (2560x1600 150%), and it looks like this:

2024-10-05-01-18-47

@terryjreedy
Copy link
Member

After my last comment, I did an experiment very similar to the PR, but was not satisfied. I decided to try using ttk.Treeview and let ttk do the spacing. I got as far as adding top-level items, with good result, before I needed to replace my system. I now have a 4k monitor and see the problem this issue is about; hence I can test fixes. Since I do not expect to have a complete replacement ready before the 3.14.0a1 release in, perhaps, 10 days, I will merge Xiaokang's first PR if requested changes are made and it then gets enough testing. Not allowing spacing to be reduced below the current value should make it fairly safe.

@Xiaokang2022
Copy link
Contributor

I've done revising it, but I'm not sure if this is what you meant.

@Xiaokang2022
Copy link
Contributor

Because the results weren't perfect on my computer. There are still some very small overlaps.

2024-10-05-03-27-46

@Wulian233
Copy link
Contributor Author

Wulian233 commented Oct 5, 2024

LGTM.
屏幕截图 2024-10-05 111047
屏幕截图 2024-10-05 111100

Also I've found that: for example, __init__.py loses the __ __, showing init. Is this a bug or a feature? If it's a bug, we can create a new issue

@Xiaokang2022
Copy link
Contributor

No, I think it's just that the underline isn't visible because of the occlusion.

@terryjreedy
Copy link
Member

Underscores, as their name implies, are at least 2 pixels down from the main bottom line of the text. They are visible for me with the patch.

terryjreedy added a commit that referenced this issue Oct 7, 2024
Increase currently inadequate vertical spacing for the IDLE browsers (path,
module, and stack) on high-resolution monitors.
---------

Co-authored-by: Terry Jan Reedy <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 7, 2024
…22392) (pythonGH-124975)

Increase currently inadequate vertical spacing for the IDLE browsers (path,
module, and stack) on high-resolution monitors.
---------

(cherry picked from commit c5df1cb)

Co-authored-by: Zhikang Yan <[email protected]>
Co-authored-by: Terry Jan Reedy <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Oct 7, 2024
…22392) (pythonGH-124975)

Increase currently inadequate vertical spacing for the IDLE browsers (path,
module, and stack) on high-resolution monitors.
---------

(cherry picked from commit c5df1cb)

Co-authored-by: Zhikang Yan <[email protected]>
Co-authored-by: Terry Jan Reedy <[email protected]>
terryjreedy added a commit that referenced this issue Oct 7, 2024
GH-124975) (#125062)

gh-122392: IDLE - Fix overlapping lines in browsers (GH-122392) (GH-124975)

Increase currently inadequate vertical spacing for the IDLE browsers (path,
module, and stack) on high-resolution monitors.
---------

(cherry picked from commit c5df1cb)

Co-authored-by: Zhikang Yan <[email protected]>
Co-authored-by: Terry Jan Reedy <[email protected]>
terryjreedy added a commit that referenced this issue Oct 7, 2024
GH-124975) (#125061)

gh-122392: IDLE - Fix overlapping lines in browsers (GH-122392) (GH-124975)

Increase currently inadequate vertical spacing for the IDLE browsers (path,
module, and stack) on high-resolution monitors.
---------

(cherry picked from commit c5df1cb)

Co-authored-by: Zhikang Yan <[email protected]>
Co-authored-by: Terry Jan Reedy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-IDLE type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

4 participants