gh-77578: IDLE Help - let users control font size#6665

csabella

Get initial font size from configuration.
Allow for dynamically changing font size via keyboard or mousewheel.

Issue: IDLE help viewer: let users control font size #77578

csabella

I have made the requested changes; please review again.

bedevere-bot

Thanks for making the requested changes!

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

terryjreedy

You can leave the one suggested code change until I retest and recheck code. 3.7.0c1 is scheduled next Tues, but I don't want to rush this without test on Mac. I really must try out devguide instructions with a small MacBook portable I was given.

terryjreedy

We started with this code, extracted from turtledemo.main.

# Menu bindings (which we might use for context menu)
        menu.add_command(label="Decrease (C-'-')", command=self.decrease_size,
                         font=menufont)
        menu.add_command(label="Increase (C-'+')", command=self.increase_size,
                         font=menufont)
# Key/mouse bindings
        text.bind_all('<%s-minus>' % shortcut, self.decrease_size)
        text.bind_all('<%s-underscore>' % shortcut, self.decrease_size)
        text.bind_all('<%s-equal>' % shortcut, self.increase_size)
        text.bind_all('<%s-plus>' % shortcut, self.increase_size)
        text.bind('<Control-MouseWheel>', self.update_mousewheel)
        text.bind('<Control-Button-4>', self.increase_size)
        text.bind('<Control-Button-5>', self.decrease_size)
# Event/action functions
darwin = sys.platform == 'darwin'

    def set_txtsize(self, size):
        txtfont[1] = size
        self.text['font'] = tuple(txtfont)
        self.output_lbl['text'] = 'Font size %d' % size

    def decrease_size(self, dummy=None):
        self.set_txtsize(max(txtfont[1] - 1, MINIMUM_FONT_SIZE))
        return 'break'

    def increase_size(self, dummy=None):
        self.set_txtsize(min(txtfont[1] + 1, MAXIMUM_FONT_SIZE))
        return 'break'

    def update_mousewheel(self, event):
        # For wheel up, event.delta = 120 on Windows, -1 on darwin.
        # X-11 sends Control-Button-4 event instead.
        if (event.delta < 0) == (not darwin):
            return self.decrease_size()
        else:
            return self.increase_size()

As far as I know, menu, keyboard, and mouse methods work on all three major systems. There are no pseudoevents. To the extent possible, the meaning of actions is determined by their bindings.

Note: the code above is extracted from 4 different locations. This is the first time I have looked at it altogether.

At my request, Cheryl combined and consolidated the functions into one zoom handler. At Tal's request, she partly undid what I had requested. Zoom() has been separated into multiple functions again. However, I prefer the original code to the result. If we don't agree on reverting to the original design, lets discuss.

terryjreedy

github threw away or hid my my review summary. Will try to find or recreate.

Now it is visible again.

csabella

When I looked back through my original commits and Terry's comments, I thought the main issue was that I was using lambda and sending another parameter to the zoom method. I didn't recall that you preferred to have just one zoom handler. Splitting them out here was a good exercise for me to get reacquainted with this change, but I don't have a preference either way. Although, splitting them out did force me to add tests for the keybindings and also I noticed I didn't test the bounds before.

taleinat

My apologies, @terryjreedy and @csabella. I wasn't aware of Terry's request to consolidate the zoom handling into a single handler. Cheryl, please do as Terry has requested, as he has the final say on these matters.

terryjreedy

Tal, I assumed that you had not gone through all the hidden 'outdated' comments. In the future, perhaps I should give better summary reviews as they should be easier to find. In any case, I welcome your review of all IDLE issues. Your dislike of zoom got me to take another look. I partly like it, but partly not. It is okay with me if we occasionally try a refactoring and decide against it.

I responded about pseudoevents and handler names on the May 17 comment on line 180.
I responded about wheel convention on yesterdays comment on line 204.

It conflicted with my requested changes.

taleinat

@terryjreedy, on the contrary, I should just take more care to read all of the previous discussion. You've got more than enough on your plate, you shouldn't bother to write summaries when the reviews are there to be read.

terryjreedy

Current requested changes: Reverse previous request to combine turtledemo functions into zoom(). However, add psuedoevents '<<increase_font_size>>' and '<<decrease_font_size>>', use the same names for the handler functions. Don't use *[] in event add. Use original mouse logic.

Use pseudoevents for testing. Don't worry about key-mouse events now.

* Remove use of callback to adjust fonts in tags. * Adjust tag font sizes the same as the main font. * TODO: Tags don't retain ratio, so after they all go to min size, they don't recover original ratios.

* Originally resized Font objects, but now can resize fonts defined as tuples.

taleinat

Ping, @csabella?

terryjreedy

I summarized 'remaining issues' on Sept 8, 2018, and added more on Sept 23 and Dec 14. I need to check all against current code updated since then.

csabella

As far as I know, I had made all the requested changes and I had tried to keep up with rebasing.

ambv

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

github-actions

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

github-actions

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

csabella requested a review from terryjreedy as a code owner April 30, 2018 23:44

the-knights-who-say-ni added the CLA signed label Apr 30, 2018

bedevere-bot added the awaiting review label Apr 30, 2018

terryjreedy changed the title ~~bpo-25198: IDLE Help: Modifications for font size~~ May 1, 2018

terryjreedy requested changes May 1, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels May 1, 2018

bedevere-bot added awaiting change review and removed awaiting changes labels May 2, 2018

terryjreedy self-assigned this May 17, 2018

terryjreedy added needs backport to 3.6 labels May 17, 2018

terryjreedy reviewed May 17, 2018

View reviewed changes

terryjreedy requested changes May 17, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels May 17, 2018

taleinat previously requested changes Aug 1, 2018

View reviewed changes

terryjreedy requested changes Sep 4, 2018

View reviewed changes

python deleted a comment from bedevere-bot Sep 5, 2018

terryjreedy requested changes Sep 5, 2018

View reviewed changes

csabella added 9 commits June 20, 2020 09:18

Move FontSizer to textview.

a23cb1c

Retain Close button when increasing font size.

e88f488

Use decorator for changing text widget

90cb041

Adjust font tag sizes within FontSizer. …

e03e4b2

* Remove use of callback to adjust fonts in tags. * Adjust tag font sizes the same as the main font. * TODO: Tags don't retain ratio, so after they all go to min size, they don't recover original ratios.

Allow for sizing fonts defined in different ways. …

645a0e0

* Originally resized Font objects, but now can resize fonts defined as tuples.

Add tests for fonts defined as tuples.

5c1d58a

Refactor return values

98295cb

Update split to splitlist

d6f80e8

Remove unrelated changes to editor.py

dab3eb5

csabella force-pushed the bpo25198 branch from 4d71d1b to dab3eb5 Compare June 20, 2020 13:19

terryjreedy added needs backport to 3.9 and removed needs backport to 3.7 labels Sep 21, 2020

terryjreedy added 3 commits September 21, 2020 08:37

Update 2018-04-30-19-11-09.bpo-25198.yzJ3SL.rst

26b3342

Merge remote-tracking branch 'upstream/master' into pr_6665

100bdea

Merge branch 'master' into bpo25198

803800d

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

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

ezio-melotti removed the label Jul 13, 2022

github-actions Bot added the stale Stale PR or inactive for long period of time. label Aug 17, 2022

terryjreedy changed the title ~~bpo-33397: IDLE Help: Modifications for font size~~ Nov 6, 2022

terryjreedy changed the title ~~gh-77578: IDLE Help: Modifications for font size~~ Nov 6, 2022

terryjreedy mentioned this pull request Nov 7, 2022

IDLE help viewer: let users control font size #77578

Open

github-actions Bot removed the stale Stale PR or inactive for long period of time. label Sep 21, 2024

github-actions Bot added the stale Stale PR or inactive for long period of time. label Dec 15, 2024

Conversation

csabella commented Apr 30, 2018 • edited by terryjreedy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

csabella commented May 2, 2018

Uh oh!

bedevere-bot commented May 2, 2018

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

terryjreedy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

terryjreedy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

csabella commented Sep 4, 2018

Uh oh!

taleinat commented Sep 5, 2018

Uh oh!

terryjreedy commented Sep 5, 2018

Uh oh!

taleinat commented Sep 5, 2018

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

taleinat commented Sep 20, 2020

Uh oh!

terryjreedy commented Sep 21, 2020

Uh oh!

csabella commented Sep 21, 2020

Uh oh!

ambv commented May 17, 2022

Uh oh!

github-actions Bot commented Aug 17, 2022

Uh oh!

github-actions Bot commented Dec 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

csabella commented Apr 30, 2018 •

edited by terryjreedy

Loading

terryjreedy left a comment •

edited

Loading

terryjreedy left a comment •

edited

Loading