Sitelet https://web.archive.org/web/20220516042821/https://github.com/python/cpython/pull/27096
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

bpo-44603: Exit the interpreter if the user types "exit" #27096

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Jul 12, 2021 •

Python/pythonrun.c Outdated Show resolved Hide resolved
Python/pythonrun.c Outdated Show resolved Hide resolved
@pablogsal pablogsal marked this pull request as ready for review Jul 12, 2021
@gpshead
Copy link
Member

@gpshead gpshead commented Jul 14, 2021

LGTM, I suggest reopening this (button clicked).

From a testing perspective, we just want to make sure it doesn't interfere with with what uber interactive console add-on's like IPython already implement that do this.

@gpshead gpshead reopened this Jul 14, 2021
@gpshead
Copy link
Member

@gpshead gpshead commented Jul 14, 2021

From a PR standpoint I think the thing we need was covered in your bug comment:

"if there's a single AST node that is a name and the name is "exit" or "quit", the REPL inspects locals and globals to see if the object referred to is a Quitter, and if so it exits"

We should be able to add unittest coverage for this in test_repl.py including equivalent test coverage that such exit/quit treatment is not present outside the repl.

@gpshead gpshead self-requested a review Jul 14, 2021
@gpshead gpshead added the type-feature label Jul 14, 2021
Copy link
Member

@terryjreedy terryjreedy left a comment •

For the purpose of the review, I will be neutral on the proposal. But I do notice that the special behavior is limited to entering exactly 'quit\n' or 'exit\n'. This is stricter than current ^Z behavior.

>>> a^Z
  File "<stdin>", line 1
    a→
     ^
SyntaxError: invalid syntax
>>> ^Zb

f:\dev\3x>

Exiting instead of also raising SyntaxError may be a bug.

Changes:

  1. The reprs of both functions should be changed to eliminate the ()s.
  2. The code class imitating the REPL should imitate this also. If no syntax error, run same tests on result as ast.parse. But do in a separate function, as here, so subclass can easily disable with def is_exit(code): return False.

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jul 17, 2021

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

}
return PyUnicode_CompareWithASCIIString(expr->v.Name.id, "exit") == 0 ||
PyUnicode_CompareWithASCIIString(expr->v.Name.id, "quit") == 0;
}
Copy link
Member

@terryjreedy terryjreedy Jul 17, 2021

Choose a reason for hiding this comment

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

Tests of the object associated with the names is needed to not turn them into semi-keywords, as in the the following.

>>> exit = 3
>>> exit

f:\dev\3x>

@github-actions
Copy link

@github-actions github-actions bot commented Aug 17, 2021

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

@github-actions github-actions bot added the stale label Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes stale type-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants