Skip to content

gh-49844: IDLE startup configuration #5541

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

gh-49844: IDLE startup configuration #5541

wants to merge 7 commits into from

Conversation

csabella
Copy link
Contributor

@csabella csabella commented Feb 5, 2018 •
edited by bedevere-bot
Loading

  • Add Startup tab to configdialog
  • Move startup options from General tab to Startup tab (and move tests)
  • Add options to Startup Tab for shell startup/restart code and editor template code (and add tests)
  • Include new options in config-main.def.
  • Add code to editor for initializing new, empty window with template code (tests needed)
  • Add code to pyshell to use new configuration on both shell startup and shell restart (tests needed)

https://bugs.python.org/issue5594

@csabella csabella requested a review from terryjreedy as a code owner February 5, 2018 01:18
@csabella
Copy link
Contributor Author

csabella commented Feb 5, 2018

I'm sorry, but I don't understand why Appveyor is failing. It's on my new tests, which are like the existing tests, only on a Text box (so it's not attached to a tracer). The failed test is showing changes as empty when the event should have caused changes to have the new value. Since this works on Travis, I'm missing what the difference between that and Appveyor would be.

terryjreedy
terryjreedy requested changes Feb 5, 2018
View reviewed changes
Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a partial review, but at least see if the text code change solves the test failure.

I do not know yet if this patch, or indeed, IDLE startup, works correctly. I may want to do some refactoring in pyshell before or as part of this change. Will comment on issue.

Lib/idlelib/pyshell.py Outdated
@@ -495,6 +495,11 @@ def restart_subprocess(self, with_cwd=False, filename=''):
# reload remote debugger breakpoints for all PyShellEditWindows
debug.load_breakpoints()
self.compile.compiler.flags = self.original_compiler_flags
if idleConf.GetOption('main', 'ShellWindow',
'restart-code-on', type='bool'):
config_startup_code = idleConf.GetOption(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, 'startup' seems fine here, perhaps because of the '_'s and the name being long anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Lib/idlelib/configdialog.py Outdated
@@ -109,11 +109,13 @@ def create_widgets(self):
self.fontpage = FontPage(note, self.highpage)
self.keyspage = KeysPage(note)
self.genpage = GenPage(note)
self.startuppage = StartupPage(note)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer 'startpage' and 'StartPage' for internal code, but leave 'Startup' on the tab itself.

Copy link
Contributor Author

@csabella csabella Feb 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Lib/idlelib/pyshell.py
if idleConf.GetOption('main', 'ShellWindow',
'startup-code-on', type='bool'):
config_startup_code = idleConf.GetOption(
'main', 'ShellWindow', 'shell-startup-code')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seems to have missed the difference between 'restart-code-on' and 'startup-code-on'. I need to look at pyshell changes, especially, in broader context. I want to understand and document the pyshell code better, with comments and docstrings , before changing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'restart-code-on' and 'startup-code-on' are checkbuttons that indicate when to run the code defined within config. Although your comment on the ticket mentions just having one code set for it, since startup and restart are different now, I added the checkbutton so that the code could be run just at initial startup, just at restart (Ctrl-F6), or both.

I thought documenting and tests for pyshell might be added as a dependency, just as the editor tests project would be a dependency.

Lib/idlelib/pyshell.py Outdated
'startup-code-on', type='bool'):
config_startup_code = idleConf.GetOption(
'main', 'ShellWindow', 'shell-startup-code')
sys.argv = ['-c'] + [config_startup_code]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I use '-c' in Windows command prompt, the string that follows must be quoted with "", not '' (not an issue when provided in a variable) and cannot contain literal newlines. I know that Linux shells are different. I suspect that the newlines in the test strings might be the test failure cause.

Even if the test passes, I would not be sure that this line will work as expected. User code must be executed by run.py in its pseudomain context after Python starts run.py in the real main context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worked on Linux and I wasn't sure what issues it would run into on other systems. Although I was considering the config code to be similar to the '-c' or -'s' options in terms of functionality. But I didn't realize that those might not currently be correct.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

@github-actions
Copy link

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

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Aug 14, 2022
@hauntsaninja hauntsaninja changed the title bpo-5594: IDLE startup configuration gh-49844: IDLE startup configuration Jan 7, 2023
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Apr 29, 2023
@github-actions GitHub Actions
Copy link

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

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label May 10, 2024
@mark-summerfield mark-summerfield mannequin mentioned this pull request Aug 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@terryjreedy terryjreedy terryjreedy requested changes

Assignees
No one assigned
Labels
awaiting changes stale Stale PR or inactive for long period of time.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@csabella @bedevere-bot @terryjreedy @the-knights-who-say-ni @ezio-melotti