Skip to content
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

[RFC] run_config(): skip interrupt unless explicitly passed config_file #551

Conversation

tony
Copy link
Contributor

@tony tony commented Sep 4, 2023

Note

I made a mistake below - making things noisier, not quieter :).

Thank you for catching the bug in my PR @orhanhenrik, and also to @jonathanslenders for the PR #563 with a quick turnaround and release.

Resolves #549

Problem

Downstream packages (e.g. django-extension's shell_plus) use ptpython.repl.run_config() to use the system's optional ptpython config files. The result is users can be surprised by run_config() interrupting the terminal for a config file they didn't explicitly request: ptpython had the default.

Current behavior

run_config() specifies a default configuration file, which may or may not exist on systems.
ptpython.repl.embed() runs flawlessly if run_config() returns an empty value.

What this change does

  1. Extracts default config_file into constant: DEFAULT_CONFIG_FILE

  2. Checks for config_file being nullish

    • If yes (nullish), set explicit_config_file to True, then:

      • Set the empty config_file to DEFAULT_CONFIG_FILE.

        Preserving default behavior

      • If no (explicit value passed), set explicit_config_file to False

  3. Check for explicit_config_file in condition that runs:

    print("Impossible to read %r" % config_file)
    enter_to_continue()
    return

Other options

@tony tony changed the title RFC: run_config() skip interrupt unless explicitly passed config_file [RFC] run_config(): skip interrupt unless explicitly passed config_file Sep 4, 2023
@jonathanslenders jonathanslenders merged commit d25e678 into prompt-toolkit:master Nov 3, 2023
@jonathanslenders
Copy link
Member

Thanks!

@tony tony deleted the run-config-without-interrupt-explicit branch November 3, 2023 15:00
@tony
Copy link
Contributor Author

tony commented Nov 3, 2023

@jonathanslenders Long time no see! Hope all is well.

@orhanhenrik
Copy link

I just tested this fix in the newest release, and it doesn't work. It is actually more noisy since it prints a traceback as well.

Traceback (most recent call last):
  File "/Users/orhan/Documents/Hyre/django-backend/app/.venv/lib/python3.11/site-packages/ptpython/repl.py", line 445, in run_config
    with open(config_file, "rb") as f:
         ^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/Users/orhan/.config/ptpython/config.py'

Press ENTER to continue...

There needs to be a return if the file doesn't exist, instead of trying to open the file.

@jonathanslenders
Copy link
Member

@orhanhenrik : I'm not using Django, so can't test right now, but looking at the code, I think the following patch could work:

--- a/ptpython/repl.py
+++ b/ptpython/repl.py
@@ -433,9 +433,10 @@ def run_config(repl: PythonInput, config_file: str | None = None) -> None:
         input("\nPress ENTER to continue...")

     # Check whether this file exists.
-    if not os.path.exists(config_file) and explicit_config_file:
-        print("Impossible to read %r" % config_file)
-        enter_to_continue()
+    if not os.path.exists(config_file):
+        if explicit_config_file:
+            print("Impossible to read %r" % config_file)
+            enter_to_continue()
         return

     # Run the config file in an empty namespace.

Would you be able to try that? @tony : Can you check as well? I can push a new release later today if this is all it takes.

@jonathanslenders
Copy link
Member

Can you check this PR: #563 ?

@tony
Copy link
Contributor Author

tony commented Dec 13, 2023

I just tested this fix in the newest release, and it doesn't work. It is actually more noisy since it prints a traceback as well.

@orhanhenrik Oh no, that's not intended. Thank you for reporting this back!

Recreated with django-extensions 3.2.4dev0 and ptpython 3.0.24:

Traceback (most recent call last):
  File ".../ptpython/ptpython/repl.py", line 445, in run_config
    with open(config_file, "rb") as f:
         ^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '~/.config/ptpython/config.py'

Press ENTER to continue...
>>>

Would you be able to try that? @tony : Can you check as well? I can push a new release later today if this is all it takes.

@jonathanslenders fix-config-file-does-not-exist / #563 works! Buttery smooth!

@jonathanslenders
Copy link
Member

Thank you for testing! The release will be for tomorrow.

@tony
Copy link
Contributor Author

tony commented Dec 13, 2023

Let's see how this goes!

@orhanhenrik
Copy link

Thanks! Looks like that fixes the issue 🙌

@jonathanslenders
Copy link
Member

3.0.25 is published to PyPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ptpython.run_config() / embed() "Impossible to read" interrupt with default usage
3 participants