Skip to content

Commit

Permalink
Fix bug on syntax errors in the interactive loop.
Browse files Browse the repository at this point in the history
';f' would first give a parse error, and then try to execute 'f'.  Now
it just gives the parse error, as expected.
  • Loading branch information
Andy Chu committed Aug 20, 2018
1 parent fd1ffce commit a967605
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 11 deletions.
23 changes: 12 additions & 11 deletions bin/oil.py
Expand Up @@ -95,22 +95,26 @@ def _tlog(msg):
_tlog('after imports')


def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader, arena):
def InteractiveLoop(opts, ex, c_parser, arena):
if opts.show_ast:
ast_f = fmt.DetectConsoleOutput(sys.stdout)
else:
ast_f = None

status = 0
while True:
# Why is this the way to handle Control-C?
# Also, we shouldn't exit the shell!
try:
w = c_parser.Peek()
except KeyboardInterrupt:
print('Ctrl-C')
break

# TODO: When does this happen?
if w is None:
raise RuntimeError('Failed parse: %s' % c_parser.Error())

c_id = word.CommandId(w)
if c_id == Id.Op_Newline:
#print('nothing to execute')
Expand All @@ -120,6 +124,7 @@ def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader, arena):
break
else:
node = c_parser.ParseCommandLine()
#log('parsed node: %s', node)

# Failed parse.
# TODO: Need an error for an empty command, which we ignore? GetLine
Expand All @@ -129,8 +134,9 @@ def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader, arena):
e = c_parser.Error()
# NOTE: This is a bit verbose.
ui.PrintErrorStack(e, arena, sys.stderr)
w_parser.Reset()

c_parser.Reset()
c_parser.ResetInputObjects()
continue

if ast_f:
Expand All @@ -143,15 +149,10 @@ def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader, arena):
if opts.print_status:
print('STATUS', repr(status))

# Reset prompt to PS1.
line_reader.Reset()

# Reset internal newline state.
# NOTE: It would actually be correct to reinitialize all objects (except
# Env) on every iteration. But we know that the w_parser is the only thing
# that needs to be reset, for now.
w_parser.Reset()
# Reset internal newline state. NOTE: It would actually be correct to
# reinitialize all objects (except Env) on every iteration.
c_parser.Reset()
c_parser.ResetInputObjects()

return status

Expand Down Expand Up @@ -311,7 +312,7 @@ def OshMain(argv0, argv, login_shell):
completion.Init(pool, builtin.BUILTIN_DEF, mem, funcs, comp_lookup,
status_out, ev)

return InteractiveLoop(opts, ex, c_parser, w_parser, line_reader, arena)
return InteractiveLoop(opts, ex, c_parser, arena)
else:
# Parse the whole thing up front
#print('Parsing file')
Expand Down
19 changes: 19 additions & 0 deletions osh/cmd_parse.py
Expand Up @@ -43,6 +43,10 @@ def __init__(self, w_parser, lexer, line_reader, arena):
self.Reset()

def Reset(self):
"""Reset our own internal state.
Called by the interactive loop.
"""
self.error_stack = []
self.completion_stack = []

Expand All @@ -54,6 +58,21 @@ def Reset(self):

self.pending_here_docs = []

def ResetInputObjects(self):
"""Reset the internal state of our inputs.
Called by the interactive loop.
TODO: Should we just make new objects on every iteration?
"""
# All the stuff we read from
self.w_parser.Reset()
#self.lexer.Reset()
# TODO: This should be a method on Lexer. But I'm not sure if we want to
# reuse objects at all.
self.lexer.line_lexer.Reset('', -1)
self.line_reader.Reset()

def Error(self):
return self.error_stack

Expand Down
4 changes: 4 additions & 0 deletions test/osh-usage.sh
Expand Up @@ -100,6 +100,10 @@ osh-interactive() {

# Parse failure
echo ';' | $OSH -i

# Bug fix: this shouldn't try execute 'echo OIL OIL'
# The line lexer wasn't getting reset on parse failures.
echo ';echo OIL OIL' | $OSH -i
}

help() {
Expand Down

0 comments on commit a967605

Please sign in to comment.