[prompt] ${x@P} shouldn't contain \x01 and \x02.

Case #1 in spec/prompt was newly failing.

- Also catch error for \]\[ (non_printing becomes negative, but ends up
  as 0)
- Make the prompt error messages more consistent
- Format to 80 cols and 2 space indent

core/comp_ui.py

@@ -30,6 +30,7 @@
 PROMPT_UNDERLINE = '\x01%s\x02' % _UNDERLINE
 PROMPT_REVERSE = '\x01%s\x02' % _REVERSE
 
+
 def _PromptLen(prompt_str):
   """Ignore all characters between \x01 and \x02."""
   escaped = False

core/comp_ui_test.py

@@ -149,17 +149,22 @@ def testDisplays(self):
       disp.ShowPromptOnRight('RIGHT')
 
 
-
 class PromptTest(unittest.TestCase):
+
   def testNoEscapes(self):
     for prompt in ["> ", "osh>", "[[]][[]][][]]][["]:
       self.assertEqual(comp_ui._PromptLen(prompt), len(prompt))
 
   def testValidEscapes(self):
-    self.assertEqual(comp_ui._PromptLen("\x01\033[01;34m\x02user\x01\033[00m\x02 >"), len("user >"))
-    self.assertEqual(comp_ui._PromptLen("\x01\x02\x01\x02\x01\x02"), 0)
-    self.assertEqual(comp_ui._PromptLen("\x01\x02 hi \x01hi\x02 \x01\x02 hello"),
-            len(" hi   hello"))
+    self.assertEqual(
+        comp_ui._PromptLen("\x01\033[01;34m\x02user\x01\033[00m\x02 >"),
+        len("user >"))
+    self.assertEqual(
+        comp_ui._PromptLen("\x01\x02\x01\x02\x01\x02"), 0)
+    self.assertEqual(
+        comp_ui._PromptLen("\x01\x02 hi \x01hi\x02 \x01\x02 hello"),
+        len(" hi   hello"))
+
 
 if __name__ == '__main__':
   unittest.main()

osh/prompt.py

@@ -23,9 +23,7 @@
 # Prompt Evaluation
 #
 
-# Global instance set by main().  TODO: Use dependency injection.
-PROMPT = None
-PROMPT_ERROR = "<error parsing prompt>"
+PROMPT_ERROR = '<Error: unbalanced \[ and \]> '
 
 # NOTE: word_compile._ONE_CHAR has some of the same stuff.
 _ONE_CHAR = {
@@ -122,6 +120,9 @@ def _ReplaceBackslashCodes(self, tokens):
 
       elif id_ == Id.PS_RBrace:
         non_printing -= 1
+        if non_printing < 0:  # e.g. \]\[
+          return PROMPT_ERROR
+
         ret.append('\x02')
 
       elif id_ == Id.PS_Subst:  # \u \h \w etc.
@@ -144,30 +145,30 @@ def _ReplaceBackslashCodes(self, tokens):
             # Shorten to ~/mydir
             r = ui.PrettyDir(val.s, self.mem.GetVar('HOME'))
           else:
-            r = '<Error: PWD is not a string>'
+            r = '<Error: PWD is not a string> '
 
         elif char == 'W':
           val = self.mem.GetVar('PWD')
           if val.tag == value_e.Str:
             r = os_path.basename(val.s)
           else:
-            r = '<Error: PWD is not a string>'
+            r = '<Error: PWD is not a string> '
 
         elif char in _ONE_CHAR:
           r = _ONE_CHAR[char]
 
         else:
-          r = '<\%s not implemented in $PS1> $' % char
+          r = '<Error: \%s not implemented in $PS1> ' % char
 
         # See comment above on bash hack for $.
         ret.append(r.replace('$', '\\$'))
 
       else:
         raise AssertionError('Invalid token %r' % id_)
 
-    # mismatched braces, see https://github.com/oilshell/oil/pull/256
+    # mismatched brackets, see https://github.com/oilshell/oil/pull/256
     if non_printing != 0:
-        return PROMPT_ERROR
+      return PROMPT_ERROR
 
     return ''.join(ret)
 

osh/prompt_test.py

@@ -14,6 +14,7 @@
 
 
 class PromptTest(unittest.TestCase):
+
   @classmethod
   def setUpClass(cls):
     arena = test_lib.MakeArena('<ui_test.py>')
@@ -31,14 +32,22 @@ def testNoEscapes(self):
       self.assertEqual(self.p.EvalPrompt(value.Str(prompt_str)), prompt_str)
 
   def testValidEscapes(self):
-    for prompt_str in ["\[\033[01;34m\]user\[\033[00m\] >", "\[\]\[\]\[\]", "\[\] hi \[hi\] \[\] hello"]:
-      self.assertEqual(self.p.EvalPrompt(value.Str(prompt_str)),
+    for prompt_str in [
+        "\[\033[01;34m\]user\[\033[00m\] >", "\[\]\[\]\[\]",
+        "\[\] hi \[hi\] \[\] hello"]:
+      self.assertEqual(
+          self.p.EvalPrompt(value.Str(prompt_str)),
           prompt_str.replace("\[", "\x01").replace("\]", "\x02"))
 
   def testInvalidEscapes(self):
-    for invalid_prompt in ["\[\[", "\[\]\[\]\]", "\]\]", "almost valid \]", "\[almost valid"]:
+    for invalid_prompt in [
+        "\[\[", "\[\]\[\]\]", "\]\]", "almost valid \]", "\[almost valid",
+        "\]\[",  # goes negative!
+        ]:
       tokens = list(match.PS1_LEXER.Tokens(invalid_prompt))
-      self.assertEqual(prompt.PROMPT_ERROR, self.p._ReplaceBackslashCodes(tokens))
+      self.assertEqual(
+          prompt.PROMPT_ERROR, self.p._ReplaceBackslashCodes(tokens))
+
 
 if __name__ == '__main__':
   unittest.main()

osh/word_eval.py

@@ -606,7 +606,9 @@ def _EvalBracedVarSub(self, part, part_vals, quoted):
       if op.tag == suffix_op_e.StringNullary:
         if op.op_id == Id.VOp0_P:
           prompt = self.prompt_ev.EvalPrompt(val)
-          val = value.Str(prompt)
+          # readline gets rid of these, so we should too.
+          p = prompt.replace('\x01', '').replace('\x02', '')
+          val = value.Str(p)
         elif op.op_id == Id.VOp0_Q:
           val = value.Str(string_ops.ShellQuote(val.s))
         else:

spec/prompt.test.sh

@@ -12,7 +12,7 @@ $ echo foo
 $ exit
 ## END
 
-#### [] are non-printing
+#### \[\] are non-printing
 PS1='\[foo\]\$'
 echo "${PS1@P}"
 ## STDOUT: