From a967605df5264cbeed90f469fc79b75f4c69a325 Mon Sep 17 00:00:00 2001 From: Andy Chu Date: Mon, 20 Aug 2018 15:41:09 -0700 Subject: [PATCH] Fix bug on syntax errors in the interactive loop. ';f' would first give a parse error, and then try to execute 'f'. Now it just gives the parse error, as expected. --- bin/oil.py | 23 ++++++++++++----------- osh/cmd_parse.py | 19 +++++++++++++++++++ test/osh-usage.sh | 4 ++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/bin/oil.py b/bin/oil.py index a241862c31..84edb34b68 100755 --- a/bin/oil.py +++ b/bin/oil.py @@ -95,7 +95,7 @@ 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: @@ -103,14 +103,18 @@ def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader, arena): 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') @@ -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 @@ -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: @@ -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 @@ -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') diff --git a/osh/cmd_parse.py b/osh/cmd_parse.py index 531c1005dc..09e7f9050c 100644 --- a/osh/cmd_parse.py +++ b/osh/cmd_parse.py @@ -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 = [] @@ -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 diff --git a/test/osh-usage.sh b/test/osh-usage.sh index e53d47818c..d68a7a4d7d 100755 --- a/test/osh-usage.sh +++ b/test/osh-usage.sh @@ -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() {