1483 - *really* check color screens in scenarios

Required fixing two levels of bugs:

a) The hack in tangle to drop initial comments a '%' directive..

b) ..was masking a bug where run_mu_scenario wasn't robust to initial
comments.

Mildly concerned that neither of the sub-issues have their own tests,
but I'm just removing hacks, and writing tests for that throwaway
function like run_mu_scenario seems pointless. Instead I've solved the
problem by disallowing comments before '%' directives.

I've also taken this opportunity to at least try to document the
'scenarios' and '%' directives at the first layer where they appear.

7 files changed, 60 insertions, 28 deletions

diff --git a/011load.cc b/011load.cc
index 8cbffbe7..b779aa9e 100644
--- a/011load.cc
+++ b/011load.cc
@@ -1,6 +1,6 @@
 //: Phase 1 of running mu code: load it from a textual representation.
 
-:(scenarios load)
+:(scenarios load)  // use 'load' instead of 'run' in all scenarios in this layer
 :(scenario first_recipe)
 recipe main [
   1:number <- copy 23:literal
diff --git a/026assert.cc b/026assert.cc
index 9e4e79df..74e85496 100644
--- a/026assert.cc
+++ b/026assert.cc
@@ -1,5 +1,5 @@
 :(scenario assert)
-% Hide_warnings = true;
+% Hide_warnings = true;  // '%' lines insert arbitrary C code into tests before calling 'run' with the lines below. Must be immediately after :(scenario) line.
 recipe main [
   assert 0:literal, [this is an assert in mu]
 ]
diff --git a/038scheduler.cc b/038scheduler.cc
index bfa316c4..f98a8fca 100644
--- a/038scheduler.cc
+++ b/038scheduler.cc
@@ -210,9 +210,9 @@ recipe f2 [
 ]
 +mem: storing 2 in location 1
 
+//: this scenario will require some careful setup in escaped C++
+//: (straining our tangle capabilities to near-breaking point)
 :(scenario scheduler_skips_completed_routines)
-# this scenario will require some careful setup in escaped C++
-# (straining our tangle capabilities to near-breaking point)
 % recipe_number f1 = load("recipe f1 [\n1:number <- copy 0:literal\n]").front();
 % recipe_number f2 = load("recipe f2 [\n2:number <- copy 0:literal\n]").front();
 % Routines.push_back(new routine(f1));  // f1 meant to run
diff --git a/050scenario.cc b/050scenario.cc
index c4acc688..06c14c8e 100644
--- a/050scenario.cc
+++ b/050scenario.cc
@@ -494,8 +494,9 @@ recipe main [
 void run_mu_scenario(const string& form) {
   istringstream in(form);
   in >> std::noskipws;
+  skip_whitespace_and_comments(in);
   string _scenario = next_word(in);
-//?   cout << _scenario << '\n'; //? 1
+//?   cout << _scenario << '\n'; //? 2
   assert(_scenario == "scenario");
   scenario s = parse_scenario(in);
   run_mu_scenario(s);
diff --git a/072scenario_screen.cc b/072scenario_screen.cc
index 6b1a1a58..98342682 100644
--- a/072scenario_screen.cc
+++ b/072scenario_screen.cc
@@ -71,7 +71,6 @@ scenario screen-in-scenario-color [
 ]
 
 :(scenario screen_in_scenario_error)
-#? % cerr << "AAA\n";
 % Hide_warnings = true;
 scenario screen-in-scenario-error [
   assume-screen 5:literal/width, 3:literal/height
@@ -87,6 +86,23 @@ scenario screen-in-scenario-error [
 ]
 +warn: expected screen location (0, 0) to contain 98 ('b') instead of 97 ('a')
 
+:(scenario screen_in_scenario_color_error)
+% Hide_warnings = true;
+# screen-should-contain can check unicode characters in the fake screen
+scenario screen-in-scenario-color [
+  assume-screen 5:literal/width, 3:literal/height
+  run [
+    screen:address <- print-character screen:address, 97:literal/a, 1:literal/red
+  ]
+  screen-should-contain-in-color 2:literal/green, [
+  #  01234
+    .a    .
+    .     .
+    .     .
+  ]
+]
++warn: expected screen location (0, 0) to be in color 2 instead of 1
+
 //: allow naming just for 'screen'
 :(before "End is_special_name Cases")
 if (s == "screen") return true;
@@ -163,7 +179,7 @@ struct raw_string_stream {
 
 :(code)
 void check_screen(const string& expected_contents, const int color) {
-//?   cerr << "Checking screen\n"; //? 1
+//?   cerr << "Checking screen for color " << color << "\n"; //? 2
   assert(!Current_routine->calls.front().default_space);  // not supported
   long long int screen_location = Memory[SCREEN];
   int data_offset = find_element_name(Type_number["screen"], "data");
@@ -185,9 +201,29 @@ void check_screen(const string& expected_contents, const int color) {
       const int cell_color_offset = 1;
       uint32_t curr = cursor.get();
       if (Memory[addr] == 0 && isspace(curr)) continue;
-      if (Memory[addr] != 0 && Memory[addr] == curr) continue;
-      if (color != -1 && color != Memory[addr+cell_color_offset]) continue;
-      // mismatch
+//?       cerr << color << " vs " << Memory[addr+1] << '\n'; //? 1
+      if (Memory[addr] != 0 && Memory[addr] == curr) {
+        if (color == -1 || color == Memory[addr+cell_color_offset]) continue;
+        // contents match but color is off
+        if (Current_scenario && !Hide_warnings) {
+          // genuine test in a mu file
+          raise << "\nF - " << Current_scenario->name << ": expected screen location (" << row << ", " << column << ") to be in color " << color << " instead of " << Memory[addr+cell_color_offset] << "'\n";
+        }
+        else {
+          // just testing check_screen
+          raise << "expected screen location (" << row << ", " << column << ") to be in color " << color << " instead of " << Memory[addr+cell_color_offset] << '\n';
+        }
+        if (!Hide_warnings) {
+          Passed = false;
+          ++Num_failures;
+        }
+        return;
+      }
+
+      // mismatch; check if we should ignore because of the color
+      if (curr == ' ' && color != -1 && color != Memory[addr+cell_color_offset]) continue;
+
+      // really a mismatch
       // can't print multi-byte unicode characters in warnings just yet. not very useful for debugging anyway.
       char expected_pretty[10] = {0};
       if (curr < 256) {
@@ -290,7 +326,7 @@ void dump_screen() {
 //?     cerr << curr << ":\n"; //? 1
     for (long long int col = 0; col < screen_width; ++col) {
       cerr << static_cast<char>(Memory[curr]);
-      ++curr;
+      curr += /*size of screen-cell*/2;
     }
     cerr << '\n';
   }
diff --git a/tangle/030tangle.cc b/tangle/030tangle.cc
index 5e25c742..4949808a 100644
--- a/tangle/030tangle.cc
+++ b/tangle/030tangle.cc
@@ -301,11 +301,6 @@ list<Line>::iterator balancing_curly(list<Line>::iterator curr) {
 void emit_test(const string& name, list<Line>& lines, list<Line>& result) {
   result.push_back(Line("TEST("+name+")", front(lines).filename, front(lines).line_number-1));  // use line number of directive
   while (!lines.empty()) {
-    // hack: drop mu comments at the start, just in case there's a '%' line after them
-    // So the tangler only passes through mu comments inside scenarios between
-    // the first input line and the last input line.
-    while (!lines.empty() && starts_with(front(lines).contents, "#"))
-      lines.pop_front();
     while (!lines.empty() && starts_with(front(lines).contents, "% ")) {
       result.push_back(Line("  "+front(lines).contents.substr(strlen("% ")), front(lines)));
       lines.pop_front();
diff --git a/tangle/030tangle.test.cc b/tangle/030tangle.test.cc
index 0d4da899..cee0ac83 100644
--- a/tangle/030tangle.test.cc
+++ b/tangle/030tangle.test.cc
@@ -221,6 +221,18 @@ void test_tangle_can_hide_warnings_in_scenarios() {
   CHECK(lines.empty());
 }
 
+void test_tangle_can_handle_in_scenarios() {
+  istringstream in(":(scenario does_bar)\n% Hide_warnings = true;\nabc def\n+layer1: pqr\n+layer2: xyz");
+  list<Line> lines;
+  tangle(in, lines);
+  CHECK_EQ(lines.front().contents, "TEST(does_bar)");  lines.pop_front();
+  CHECK_EQ(lines.front().contents, "  Hide_warnings = true;");  lines.pop_front();
+  CHECK_EQ(lines.front().contents, "  run(\"abc def\\n\");");  lines.pop_front();
+  CHECK_EQ(lines.front().contents, "  CHECK_TRACE_CONTENTS(\"layer1: pqrlayer2: xyz\");");  lines.pop_front();
+  CHECK_EQ(lines.front().contents, "}");  lines.pop_front();
+  CHECK(lines.empty());
+}
+
 void test_tangle_supports_strings_in_scenarios() {
   istringstream in(":(scenario does_bar)\nabc \"def\"\n+layer1: pqr\n+layer2: \"xyz\"");
   list<Line> lines;
@@ -315,18 +327,6 @@ void test_tangle_can_handle_mu_comments_in_scenario() {
   CHECK(lines.empty());
 }
 
-void test_tangle_can_handle_escaped_setup_after_mu_comments() {
-  istringstream in(":(scenario does_bar)\n# comment\n% int x = 1;\nabc\n+layer1: pqr\n");
-  list<Line> lines;
-  tangle(in, lines);
-  CHECK_EQ(lines.front().contents, "TEST(does_bar)");  lines.pop_front();
-  CHECK_EQ(lines.front().contents, "  int x = 1;"); lines.pop_front();
-  CHECK_EQ(lines.front().contents, "  run(\"abc\\n\");");  lines.pop_front();
-  CHECK_EQ(lines.front().contents, "  CHECK_TRACE_CONTENTS(\"layer1: pqr\");");  lines.pop_front();
-  CHECK_EQ(lines.front().contents, "}");  lines.pop_front();
-  CHECK(lines.empty());
-}
-
 
 
 void test_trim() {