Skip to content

refactor(repl): optimize shell_exec and REPL loop for clarity and eff… #6058

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

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

refactor(repl): optimize shell_exec and REPL loop for clarity and eff… #6058

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
225 changes: 79 additions & 146 deletions src/shell.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
mod helper;

use rustpython_compiler::{
CompileError, ParseError, parser::FStringErrorType, parser::LexicalErrorType,
parser::ParseErrorType,
CompileError, ParseError, parser::{FStringErrorType, LexicalErrorType, ParseErrorType}
};
use rustpython_vm::{
AsObject, PyResult, VirtualMachine,
builtins::PyBaseExceptionRef,
compiler::{self},
readline::{Readline, ReadlineResult},
scope::Scope,
AsObject, PyResult, VirtualMachine, builtins::PyBaseExceptionRef,
compiler, readline::{Readline, ReadlineResult}, scope::Scope,
};

enum ShellExecResult {
Expand All @@ -26,211 +22,148 @@ fn shell_exec(
empty_line_given: bool,
continuing_block: bool,
) -> ShellExecResult {
// compiling expects only UNIX style line endings, and will replace windows line endings
Copy link
Member

Choose a reason for hiding this comment

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

is this comment outdated?

// internally. Since we might need to analyze the source to determine if an error could be
// resolved by future input, we need the location from the error to match the source code that
// was actually compiled.
#[cfg(windows)]
let source = &source.replace("\r\n", "\n");

match vm.compile(source, compiler::Mode::Single, "<stdin>".to_owned()) {
Ok(code) => {
if empty_line_given || !continuing_block {
// We want to execute the full code
match vm.run_code_obj(code, scope) {
Ok(_val) => ShellExecResult::Ok,
Err(err) => ShellExecResult::PyErr(err),
}
vm.run_code_obj(code, scope)
.map(|_| ShellExecResult::Ok)
.unwrap_or_else(ShellExecResult::PyErr)
} else {
// We can just return an ok result
ShellExecResult::Ok
}
}
Err(CompileError::Parse(ParseError {
error: ParseErrorType::Lexical(LexicalErrorType::Eof),
..
})) => ShellExecResult::ContinueLine,
Err(CompileError::Parse(ParseError {
error:
ParseErrorType::Lexical(LexicalErrorType::FStringError(
FStringErrorType::UnterminatedTripleQuotedString,
)),
..
})) => ShellExecResult::ContinueLine,
Err(err) => {
// Check if the error is from an unclosed triple quoted string (which should always
// continue)
if let CompileError::Parse(ParseError {
error: ParseErrorType::Lexical(LexicalErrorType::UnclosedStringError),
raw_location,
..
}) = err
{
let loc = raw_location.start().to_usize();
let mut iter = source.chars();
if let Some(quote) = iter.nth(loc) {
if iter.next() == Some(quote) && iter.next() == Some(quote) {

Err(CompileError::Parse(ParseError { error, raw_location, .. })) => {
use LexicalErrorType::*;
use ParseErrorType::*;

match &error {
Lexical(Eof) |
Lexical(FStringError(FStringErrorType::UnterminatedTripleQuotedString)) => {
ShellExecResult::ContinueLine
}

Lexical(UnclosedStringError) => {
let loc = raw_location.start().to_usize();
let mut iter = source.chars().skip(loc);
if iter.next().map_or(false, |q| iter.next() == Some(q) && iter.next() == Some(q)) {
return ShellExecResult::ContinueLine;
}
ShellExecResult::ContinueBlock
}
};

// bad_error == true if we are handling an error that should be thrown even if we are continuing
// if its an indentation error, set to true if we are continuing and the error is on column 0,
// since indentations errors on columns other than 0 should be ignored.
// if its an unrecognized token for dedent, set to false

let bad_error = match err {
CompileError::Parse(ref p) => {
match &p.error {
ParseErrorType::Lexical(LexicalErrorType::IndentationError) => {
continuing_block
} // && p.location.is_some()
ParseErrorType::OtherError(msg) => {
if msg.starts_with("Expected an indented block") {
continuing_block
} else {
true
}
}
_ => true, // !matches!(p, ParseErrorType::UnrecognizedToken(Tok::Dedent, _))

Lexical(IndentationError) => {
if continuing_block {
ShellExecResult::PyErr(vm.new_syntax_error(&CompileError::Parse(ParseError {
error,
raw_location,
}), Some(source)))
} else {
ShellExecResult::ContinueBlock
}
}
_ => true, // It is a bad error for everything else
};

// If we are handling an error on an empty line or an error worthy of throwing
if empty_line_given || bad_error {
ShellExecResult::PyErr(vm.new_syntax_error(&err, Some(source)))
} else {
ShellExecResult::ContinueBlock
OtherError(msg) if msg.starts_with("Expected an indented block") => {
if continuing_block {
ShellExecResult::PyErr(vm.new_syntax_error(&CompileError::Parse(ParseError {
error,
raw_location,
}), Some(source)))
} else {
ShellExecResult::ContinueBlock
}
}

_ if empty_line_given => {
ShellExecResult::PyErr(vm.new_syntax_error(&CompileError::Parse(ParseError {
error,
raw_location,
}), Some(source)))
}

_ => ShellExecResult::ContinueBlock,
}
}

Err(err) => {
ShellExecResult::PyErr(vm.new_syntax_error(&err, Some(source)))
}
}
}

/// Enter a repl loop
pub fn run_shell(vm: &VirtualMachine, scope: Scope) -> PyResult<()> {
let mut repl = Readline::new(helper::ShellHelper::new(vm, scope.globals.clone()));
let mut full_input = String::new();

// Retrieve a `history_path_str` dependent on the OS
let repl_history_path = match dirs::config_dir() {
Some(mut path) => {
path.push("rustpython");
path.push("repl_history.txt");
path
}
None => ".repl_history.txt".into(),
};
let repl_history_path = dirs::config_dir()
.map(|mut path| { path.push("rustpython/repl_history.txt"); path })
Copy link
Member

Choose a reason for hiding this comment

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

Is this also windows(backslash) compatible?

.unwrap_or_else(|| ".repl_history.txt".into());

if repl.load_history(&repl_history_path).is_err() {
println!("No previous history.");
}

// We might either be waiting to know if a block is complete, or waiting to know if a multiline
// statement is complete. In the former case, we need to ensure that we read one extra new line
// to know that the block is complete. In the latter, we can execute as soon as the statement is
// valid.
let mut continuing_block = false;
let mut continuing_line = false;

loop {
let prompt_name = if continuing_block || continuing_line {
"ps2"
} else {
"ps1"
};
let prompt = vm
.sys_module
.get_attr(prompt_name, vm)
.and_then(|prompt| prompt.str(vm));
let prompt = match prompt {
Ok(ref s) => s.as_str(),
Err(_) => "",
};
let prompt_name = if continuing_block || continuing_line { "ps2" } else { "ps1" };
let prompt = vm.sys_module.get_attr(prompt_name, vm)
.and_then(|p| p.str(vm)).map(|s| s.as_str()).unwrap_or("");

continuing_line = false;
let result = match repl.readline(prompt) {
match repl.readline(prompt) {
ReadlineResult::Line(line) => {
#[cfg(debug_assertions)]
debug!("You entered {line:?}");

repl.add_history_entry(line.trim_end()).unwrap();

let empty_line_given = line.is_empty();
repl.add_history_entry(line.trim_end()).ok();

let empty_line = line.trim().is_empty();
if full_input.is_empty() {
full_input = line;
} else {
full_input.push_str(&line);
}
full_input.push('\n');

match shell_exec(
vm,
&full_input,
scope.clone(),
empty_line_given,
continuing_block,
) {
match shell_exec(vm, &full_input, scope.clone(), empty_line, continuing_block) {
ShellExecResult::Ok => {
if continuing_block {
if empty_line_given {
// We should exit continue mode since the block successfully executed
continuing_block = false;
full_input.clear();
}
} else {
// We aren't in continue mode so proceed normally
if !continuing_block || empty_line {
full_input.clear();
continuing_block = false;
}
Ok(())
}
// Continue, but don't change the mode
ShellExecResult::ContinueLine => {
continuing_line = true;
Ok(())
}
ShellExecResult::ContinueBlock => {
continuing_block = true;
Ok(())
}
ShellExecResult::ContinueLine => continuing_line = true,
ShellExecResult::ContinueBlock => continuing_block = true,
ShellExecResult::PyErr(err) => {
continuing_block = false;
full_input.clear();
Err(err)
vm.print_exception(err);
}
}
}

ReadlineResult::Interrupt => {
continuing_block = false;
full_input.clear();
let keyboard_interrupt =
vm.new_exception_empty(vm.ctx.exceptions.keyboard_interrupt.to_owned());
Err(keyboard_interrupt)
}
ReadlineResult::Eof => {
break;
}
ReadlineResult::Other(err) => {
eprintln!("Readline error: {err:?}");
break;
let interrupt = vm.new_exception_empty(vm.ctx.exceptions.keyboard_interrupt.clone());
vm.print_exception(interrupt);
}

ReadlineResult::Eof => break,

ReadlineResult::Other(err) |
ReadlineResult::Io(err) => {
eprintln!("IO error: {err:?}");
eprintln!("Readline error: {err:?}");
break;
}
};

if let Err(exc) = result {
if exc.fast_isinstance(vm.ctx.exceptions.system_exit) {
repl.save_history(&repl_history_path).unwrap();
return Err(exc);
}
vm.print_exception(exc);
}
}
repl.save_history(&repl_history_path).unwrap();

repl.save_history(&repl_history_path).unwrap();
Ok(())
}
Loading