bpo-5680: IDLE: Customize running a module.#13763

csabella

The initialize options are 1) add command line options, which are appended to sys.argv as if passed on a real command line, and 2) skip the shell restart. The customization dialog is accessed by a new item on the Run menu.

https://bugs.python.org/issue5680

terryjreedy

Test of behavior without looking much at code yet. Will respond to other comments later. Immediate issue: I have a custom keyset without new pseudoevent binding and on startup see

Warning: config.py - IdleConf.GetCoreKeys -
problem retrieving key binding for event '<>'
from key set 'test'.
returning default value: ['']

The fix is simple and I know how to do it if you don't beat me.

Separate issue if not done yet: write back keyset with new bindings.

Otherwise, works great.

bedevere-bot

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

taleinat

I find the name "Run Custom" unclear and confusing, and I image some users would too. Is "Run Module With Arguments" too long? Perhaps "Run With Arguments..."?
It would be great to remember the last used arguments for each file, and pre-populate the field with them.

csabella

I have made the requested changes; please review again.

@terryjreedy, I added this pseudoevent to the former_extension_events set. I think that's what you wanted to fix -- to suppress the warning for the new binding and then to work on updating any custom keysets in another ticket. Maybe former_extension_events needs to be renamed?

bedevere-bot

Thanks for making the requested changes!

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

csabella

Thanks for looking at this, @taleinat!

I find the name "Run Custom" unclear and confusing, and I image some users would too. Is "Run Module With Arguments" too long? Perhaps "Run With Arguments..."?

Terry wants to have more things on that dialog to be able to change the run environment, so using 'arguments' on the menu isn't inclusive enough. If 'Run Custom' is awkward, maybe 'Customize Run'?

It would be great to remember the last used arguments for each file, and pre-populate the field with them.

Yes, there was a few things that I listed on the bpo issue that I didn't do in this first round, including saving the values. Terry requested creating a minimal version to start with. Saving the items wouldn't (shouldn't?) complicate the code, but I wanted to try to keep it simple and then iterate over it in the next round. He had also mentioned printing the run options in the shell window so that multiple runs would be annotated. Both features would be helpful.

taleinat

I find the name "Run Custom" unclear and confusing, and I image some users would too. Is "Run Module With Arguments" too long? Perhaps "Run With Arguments..."?

Terry wants to have more things on that dialog to be able to change the run environment, so using 'arguments' on the menu isn't inclusive enough. If 'Run Custom' is awkward, maybe 'Customize Run'?

I see. Perhaps just "Run..."?

taleinat

As a general note, I'm not sure about the usefulness of the non-GUI tests for the RunCustom dialog (CustomRunCLIargsokTest, CustomRunEntryokTest) . They rely rather heavily on the internal implementation. Since we have GUI tests run by the CI now, I prefer to have simpler tests actually using Tk with less mocking. Would it be possible to simply move the tests from these classes into CustomRunGuiTest?

terryjreedy

I am reviewing and editing with the intention of getting this into 3.7.4. While the user UI can be changed after that, it is my primary focus. Summary responses to comments above.

Config warning: fixed.
Menu entry: We could flip 'Run custom', but I prefer to keep to a verb form. ('Python Shell' might better be 'Open Shell'.) I think 'Run customized' or 'Run ... customized' as an abbreviation for 'Run module with customized settings' is better, and should be good enough for now. I will see how latter looks.
Restart (or not) button: now functional. Add to blurb.
Arguments, without restart: These extend sys.argv. 'Command line' arguments do not make sense without restart, but extending sys.argv does. So this is OK.
No arguments, with restart: I changed my mind and will remove the current error check. No args will be ok with other settings, and if someone opens box to see available customizations, I now think they should be able to run without changing anything.
(Note: check button will be replaced with radio buttons when another mutually exclusive option is added, such as 'O Run module in system console' (no issue yet).)
Code comments, especially tests: I will make the changes I currently agree with.
Blurb: will expand.

Revise corresponding doc entry.

terryjreedy

buglet to fix: focus is not being shifted to shell correctly.

Don't ask for config if still may toss. Want focus to go to shell unless cancel.

terryjreedy

Changes: 1) Check code before bother to customize. 2) Add module title to title bar. 3) Make shell.text the parent, so focus goes to shell. Good enough to merge. See issue for 2 remaining issues.

miss-islington

Thanks @csabella for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington

Thanks @csabella for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

The initialize options are 1) add command line options, which are appended to sys.argv as if passed on a real command line, and 2) skip the shell restart. The customization dialog is accessed by a new entry on the Run menu. (cherry picked from commit 201bc2d) Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>

bedevere-bot

GH-14184 is a backport of this pull request to the 3.7 branch.

bedevere-bot

GH-14185 is a backport of this pull request to the 3.8 branch.

The initialize options are 1) add command line options, which are appended to sys.argv as if passed on a real command line, and 2) skip the shell restart. The customization dialog is accessed by a new entry on the Run menu. (cherry picked from commit 201bc2d) Co-authored-by: Cheryl Sabella <cheryl.sabella@gmail.com>

csabella

@taleinat, thank you for the review and @terryjreedy, thank you for the review and making the additional changes.

The initialize options are 1) add command line options, which are appended to sys.argv as if passed on a real command line, and 2) skip the shell restart. The customization dialog is accessed by a new entry on the Run menu.

bpo-5680: IDLE: Run modules with command line arguments

ffc22b8

csabella requested a review from terryjreedy as a code owner June 3, 2019 00:37

the-knights-who-say-ni added the CLA signed label Jun 3, 2019

bedevere-bot added the awaiting core review label Jun 3, 2019

blurb-it Bot and others added 2 commits June 3, 2019 00:39

📜🤖 Added by blurb_it.

36f7771

Fix editing error

69cf1b6

brettcannon added the type-feature A feature request or enhancement label Jun 3, 2019

Create more generic dialog and change keybinding

dbdd580

csabella commented Jun 3, 2019

View reviewed changes

Forkest mentioned this pull request Jun 4, 2019

bpo-5680: IDLE: Add run module with arguments to Key-F7 in editor #1589

Closed

terryjreedy requested changes Jun 6, 2019

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting core review labels Jun 6, 2019

Don't show warning message for new key binding

f1e7c61

bedevere-bot removed the awaiting changes label Jun 6, 2019

bedevere-bot added the awaiting change review label Jun 6, 2019

taleinat reviewed Jun 11, 2019

View reviewed changes

Merge remote-tracking branch 'origin/master' into pr_13763

286e4c6

Change menu entry 'Run Custom' to 'Run ... Customized'. …

03e0f59

Revise corresponding doc entry.

terryjreedy added 3 commits June 16, 2019 01:59

Move some test dummy attributes to instance.

845178b

A couple more.

31416fd

Update query coverage.

fbfca23

terryjreedy approved these changes Jun 16, 2019

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Jun 16, 2019

pass rub_args by keyword

4da67c3

terryjreedy added 3 commits June 17, 2019 18:23

Merge branch 'master' into bpo5680

307ac6a

Show customize box after check, attach to shell. …

4a64274

Don't ask for config if still may toss. Want focus to go to shell unless cancel.

Minor tweaks.

18177da

terryjreedy changed the title ~~bpo-5680: IDLE: Run modules with command line arguments~~ Jun 18, 2019

terryjreedy merged commit 201bc2d into python:master Jun 18, 2019

bedevere-bot removed the awaiting merge label Jun 18, 2019

terryjreedy added needs backport to 3.7 labels Jun 18, 2019

bedevere-bot removed the needs backport to 3.7 label Jun 18, 2019

bedevere-bot removed the needs backport to 3.8 label Jun 18, 2019

csabella deleted the bpo5680 branch June 20, 2019 21:59

Conversation

csabella commented Jun 3, 2019 • edited by terryjreedy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jun 6, 2019

Uh oh!

taleinat commented Jun 6, 2019

Uh oh!

csabella commented Jun 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Jun 6, 2019

Uh oh!

csabella commented Jun 6, 2019

Uh oh!

taleinat commented Jun 7, 2019

Uh oh!

taleinat left a comment

Choose a reason for hiding this comment

Uh oh!

terryjreedy commented Jun 16, 2019

Uh oh!

terryjreedy commented Jun 16, 2019

Uh oh!

terryjreedy commented Jun 18, 2019

Uh oh!

miss-islington commented Jun 18, 2019

Uh oh!

miss-islington commented Jun 18, 2019

Uh oh!

bedevere-bot commented Jun 18, 2019

Uh oh!

bedevere-bot commented Jun 18, 2019

Uh oh!

csabella commented Jun 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

csabella commented Jun 3, 2019 •

edited by terryjreedy

Loading

csabella commented Jun 6, 2019 •

edited

Loading