hylang / hy

A dialect of Lisp that's embedded in Python
http://hylang.org
Other
5.11k stars 372 forks source link

Fix non-interactive with param -i in subprocess #2557

Closed a690700752 closed 4 months ago

a690700752 commented 6 months ago

continue #2555 . When I use the -i parameter to force the launch of the REPL and send code in subprocess (using nvim conjure), it directly enters the repl.runsource block for execution and never enters repl.run(). In this case, the current environment is not the REPL environment, lacking the PS1, PS2 prompts, and some other REPL-specific environment settings, which contradicts the meaning of the -i parameter. On the other hand, other branches execute code first, such as from a file, before entering repl.run().

Kodiologist commented 6 months ago

When making further changes to this PR, please add commits (or force-push) rather than closing it and making a new one.

Kodiologist commented 6 months ago

The test is a bit wonky. Giving the default argument to getattr means that you're accepting either of sys.ps1 or sys.flags.interactive. Shouldn't you be more specific as to which you expect, or simply test for both?

a690700752 commented 6 months ago

There just a tests result, below is the result. Maybe we can add comment to that test code.

Environment IPython PyCharm Console Shell (run Python, paste code) Jupyter Notebook PyCharm Run PyCharm Run (Emulate terminal) Shell (run Python file)
Expected Value TRUE TRUE TRUE TRUE FALSE FALSE FALSE
hasattr(sys,'ps1') TRUE TRUE TRUE TRUE FALSE FALSE FALSE
hasattr(sys,'ps2') TRUE TRUE TRUE TRUE FALSE FALSE FALSE
sys.__stdin__.isatty() TRUE FALSE TRUE FALSE FALSE TRUE TRUE
sys.__stdout__.isatty() TRUE FALSE TRUE TRUE FALSE TRUE TRUE
hasattr(__builtins__,'__IPYTHON__') TRUE FALSE FALSE TRUE FALSE FALSE FALSE
not hasattr(__main**, '__file__') TRUE FALSE TRUE TRUE FALSE FALSE TRUE
bool(sys.flags.interactive) FALSE FALSE FALSE FALSE FALSE FALSE FALSE
Kodiologist commented 6 months ago

That's an impressive table you've compiled, but your test only checks hy -i, so that's what the assertion should be written for.

a690700752 commented 6 months ago

Ok, I'll continue finish tests.

Kodiologist commented 5 months ago

How's this going?

Kodiologist commented 4 months ago

Superseded by #2587.