prompt-toolkit / ptpython

A better Python REPL
BSD 3-Clause "New" or "Revised" License
5.19k stars 277 forks source link

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

Closed tony closed 11 months ago

tony commented 1 year ago

[!NOTE] Oh look, 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

jonathanslenders commented 11 months ago

Thanks!

tony commented 11 months ago

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

orhanhenrik commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

Can you check this PR: https://github.com/prompt-toolkit/ptpython/pull/563 ?

tony commented 9 months ago

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 / https://github.com/prompt-toolkit/ptpython/pull/563 works! Buttery smooth!

jonathanslenders commented 9 months ago

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

tony commented 9 months ago

Let's see how this goes!

orhanhenrik commented 9 months ago

Thanks! Looks like that fixes the issue 🙌

jonathanslenders commented 9 months ago

3.0.25 is published to PyPI.