[history] Avoid conflict with ${!indirect} and [!charclass].

Add unit tests for conflicts with $!x and !(foo|bar).

This is a simpler hack than the one bash has.

Fixes issue #264.

Also:

- Fix unit tests broken by the PWD change.

Author: Andy Chu

frontend/lex.py

@@ -610,7 +610,11 @@ def IsKeyword(name):
   # Search by prefix of substring (optional '?').
   # NOTE: there are no numbers allowed here!  Bash doesn't seem to support it.
   # No hyphen since it conflits with $-1 too.
-  R(r'!\??[a-zA-Z_/.][0-9a-zA-Z_/.]+', Id.History_Search),
+  # 
+  # Required trailing whitespace is there to avoid conflict with [!charclass]
+  # and ${!indirect}.  This is a simpler hack than the one bash has.  See
+  # frontend/lex_test.py.
+  R(r'!\??[a-zA-Z_/.][0-9a-zA-Z_/.]+[ \t\r\n]', Id.History_Search),
 
   # Single quoted, e.g. 'a' or $'\n'.  Terminated by another single quote or
   # end of string.

frontend/lex_test.py

@@ -342,6 +342,39 @@ def testHistoryLexer(self):
     # YES operator
     self.assert_(Id.History_Op in [tok_type for tok_type, _ in tokens])
 
+  def testHistoryDoesNotConflict(self):
+    lex = match.HISTORY_LEXER
+
+    # https://github.com/oilshell/oil/issues/264
+    #
+    # Bash has a bunch of hacks to suppress the conflict between ! for history
+    # and:
+    #
+    # 1. [!abc] globbing
+    # 2. ${!foo} indirect expansion
+    # 3. $!x -- the PID
+    # 4. !(foo|bar) -- extended glob
+    #
+    # I guess [[ a != b ]] doesn't match the pattern in bash.
+
+    three_other = [Id.History_Other, Id.History_Other, Id.History_Other]
+    two_other = [Id.History_Other, Id.History_Other]
+    CASES = [
+        (r'[!abc]',       three_other),
+        (r'${!indirect}', three_other),
+        (r'$!x',          three_other),  # didn't need a special case
+        (r'!(foo|bar)',   two_other),  # didn't need a special case
+    ]
+
+    for s, expected_types in CASES:
+      tokens = list(lex.Tokens(s))
+      print(tokens)
+      actual_types = [id_ for id_, val in tokens]
+
+      self.assert_(Id.History_Search not in actual_types, tokens)
+
+      self.assertEqual(expected_types, actual_types)
+
 
 if __name__ == '__main__':
   unittest.main()

osh/builtin.py

@@ -666,8 +666,6 @@ def _PrintDirStack(dir_stack, style, home_dir):
       print(ui.PrettyDir(entry, home_dir))
 
   elif style == SINGLE_LINE:
-    # NOTE: Changed to list comprehension to avoid LOAD_CLOSURE/MAKE_CLOSURE.
-    # TODO: Restore later?
     s = ' '.join(ui.PrettyDir(entry, home_dir) for entry in dir_stack.Iter())
     print(s)
 

osh/history.py

@@ -133,6 +133,11 @@ def Eval(self, line):
           raise util.HistoryError('%s: not found', val)
 
       elif id_ == Id.History_Search:
+        # Remove the required space at the end and save it.  A simple hack than
+        # the one bash has.
+        last_char = val[-1]
+        val = val[:-1]
+
         # Search backward
         prefix = None
         substring = None
@@ -149,6 +154,7 @@ def Eval(self, line):
           if substring and substring in cmd:
             out = cmd
           if out is not None:
+            out += last_char  # restore required space
             break
 
         if out is None:

osh/history_test.py

@@ -56,9 +56,9 @@ def testReplacements(self):
     self.assertEqual('echo hi', hist_ev.Eval('echo hi'))
 
     # Search for prefix
-    self.assertEqual('echo ${two:-}', hist_ev.Eval('!echo'))
+    self.assertEqual('echo ${two:-}\n', hist_ev.Eval('!echo\n'))
     # Search for substring
-    self.assertEqual('echo ${two:-}', hist_ev.Eval('!?two'))
+    self.assertEqual('echo ${two:-} ', hist_ev.Eval('!?two '))
 
     # Indexes and negative indexes
     self.assertEqual('echo 1', hist_ev.Eval('!1'))

osh/state_test.py

@@ -99,7 +99,8 @@ def testSetVarClearFlag(self):
     self.assertEqual('/', mem.var_stack[0].vars['PYTHONPATH'].val.s)
     self.assertEqual(True, mem.var_stack[0].vars['PYTHONPATH'].exported)
 
-    self.assertEqual({'PYTHONPATH': '/'}, mem.GetExported())
+    ex = mem.GetExported()
+    self.assertEqual('/', ex['PYTHONPATH'])
 
     mem.SetVar(
         lvalue.LhsName('PYTHONPATH'), None, (var_flags_e.Exported,),
@@ -242,7 +243,7 @@ def testExportThenAssign(self):
         lvalue.LhsName('U'), value.Str('u'), (), scope_e.Dynamic)
     print(mem)
     e = mem.GetExported()
-    self.assertEqual({'U': 'u'}, e)
+    self.assertEqual('u', e['U'])
 
   def testUnset(self):
     mem = _InitMem()