Implement suggestions from the code review of the pull request #103.

Console/ConsoleTest.cc

@@ -10,7 +10,7 @@ int main()
 {
     retro::InitConsole();
     std::string out = "\033]0;Hello world win\007Hello, \033[1mexternal world of \033[0m\033[3mtrue beauty and \033[4mgreatness\033[0m.\nEnter \"exit\" to quit.\n";
-    //std::string out = "Hello, \033[1mexternal world of \033[0m\033[3mtrue beauty and \033[4mgreatness\033[0m.\nEnter \"exit\" to quit.\n";
+
     retro::Console::currentInstance->write(out.data(), out.size());
 
     std::string in;

Console/readme.txt

@@ -1,17 +1,18 @@
-    IConsole
+    Console
 
-This is a slightly improved version of Retro68's Console library, offering a
+This library provides a simple console library, offering a limited support for ANSI control sequences.
-support for a subset of ANSI control sequences.
 
 Here is a list of the supported sequences and their meaning:
 
-ESC[0m          Reset all text style
+ESC[0m              Reset all text style
-ESC[1m          Bold font (*)
+ESC[1m              Bold font (*)
-ESC[3m          Italic font
+ESC[3m              Italic font
-ESC[4m          Underline font
+ESC[4m              Underline font
 
+ESC]0;NewnameBEL    Set the window title to "Newname" (**)
 
 NOTES:
 (*) Obtained with bold + condense style together, so that the character grid
 remains regular (this way, the width of bold characters remains the same as 
 regular ones).
+(**) BEL is ASCII character 7.

Console/retro/Console.cc

@@ -30,6 +30,9 @@
 
 using namespace retro;
 
+const char BEL = 7;
+const char MAX_LEN = 250;
+
 Console *Console::currentInstance = NULL;
 
 Attributes::Attributes(void)
@@ -162,7 +165,7 @@ void Console::Init(GrafPtr port, Rect r)
     onscreen = chars;
 
     cursorX = cursorY = 0;
-    isProcessingEscSequence=false;
+    sequenceState=State::noSequence;
 }
 
 void Console::SetAttributes(Attributes aa)
@@ -265,82 +268,22 @@ void Console::ScrollUp(short n)
     dirtyRect.bottom = dirtyRect.bottom > 0 ? dirtyRect.bottom - 1 : 0;
 }
 
-void Console::ProcessOSCseq(char c)
+bool Console::ProcessEscSequence(char c)
 {
-    switch(sequenceStep)
+    switch(sequenceState)
     {
-    case 1:
+    case State::noSequence:
-        if(c!='0')  // The only recognized sequence is OSC 0;
+        return false;                       // Break is not needed there.
-        {
+    case State::waitingForSequenceStart:
-            OSCseq=false;
+        if(c=='[')
-            sequenceStep=0;
+            sequenceState=State::waitingForControlSequence;
-            return;
+        else if(c==']')
-        }
+            sequenceState=State::waitingForOSCStart;
-        ++sequenceStep;
-        break;
-    case 2:
-        if(c!=';')  // The only recognized sequence is OSC 0;
-        {
-            OSCseq=false;
-            sequenceStep=0;
-            return;
-        }
-        ++sequenceStep;
-        windowName="";
-        break;
-    default:
-        if(c==BEL) // The BEL character ends the sequence.
-        {
-            setWindowName(windowName);
-            OSCseq=false;
-            isProcessingEscSequence=false;
-            sequenceStep=0;
-        }
         else
-        {
+            sequenceState=State::noSequence;   // Unrecognized sequence
-            windowName+=c;
-            ++sequenceStep;
-        }
-    }
-}
-
-void Console::ProcessEscSequence(char c)
-{
-    if(sequenceStep>0 && OSCseq)
-    {
-        ProcessOSCseq(c);
-        //sequenceStep=0;
-        //isProcessingEscSequence=false;
-        return;
-    }
-    if(sequenceStep>MAX_LEN)
-    {
-        // Sequence is too long!
-        sequenceStep=0;
-        isProcessingEscSequence=false;
-        return;
-    }
-
-    switch(sequenceStep)
-    {
-    case 0:
-        if(c=='[')  // Control Sequence Introducer
-        {
-            OSCseq=false;
-            ++sequenceStep;
-        }
-        else if(c==']') // Operating System Command
-        {
-            OSCseq=true;
-            ++sequenceStep;
-        }
-        else
-        {
-            isProcessingEscSequence=false;
-        }
         break;
-    case 1:
+    case State::waitingForControlSequence:
-        ++sequenceStep;
+        sequenceState=State::waitingForM;
         switch(c)
         {
         case '0':   // Normal character
@@ -356,36 +299,61 @@ void Console::ProcessEscSequence(char c)
             currentAttr.setUnderline(true);
             break;
         default:
-            isProcessingEscSequence=false;
+            sequenceState=State::noSequence;   // Unrecognized sequence
         }
         break;
-    case 2:
+    case State::waitingForM:
         if(c=='m')
-            isProcessingEscSequence=false;
+            sequenceState=State::noSequence;   // Normal end of sequence
-        else if(c==';')
-            sequenceStep=1;
         else
-            isProcessingEscSequence=false;
+            sequenceState=State::noSequence;   // Unrecognized sequence (but we end it anyway!)
+        break;
+    case State::waitingForOSCStart:
+        if(c=='0')
+            sequenceState=State::waitingForSemicolon;
+        else
+            sequenceState=State::noSequence;   // Normal end of sequence
+        break;
+    case State::waitingForSemicolon:
+        if(c==';')
+        {
+            sequenceState=State::inWindowName;
+            windowName="";
+        }
+        else
+            sequenceState=State::noSequence;   // Normal end of sequence
+        break;
+    case State::inWindowName:
+        if(c==BEL)
+        {
+            setWindowName(std::move(windowName));
+            sequenceState=State::noSequence;   // Normal end of sequence
+        }
+        else
+        {
+            if(windowName.size() < MAX_LEN)
+                windowName+=c;
+        }
         break;
     default:
-        sequenceStep=0;
+        sequenceState=State::noSequence;
-        isProcessingEscSequence=false;
+        break;
     }
+    return true;
 }
 
 void Console::PutCharNoUpdate(char c)
 {
-    if(isProcessingEscSequence)
+    if(ProcessEscSequence(c))
-    {
-        ProcessEscSequence(c);
         return;
-    }
+
     InvalidateCursor();
     switch(c)
     {
     case '\033':    // Begin of an ANSI escape sequence
-        isProcessingEscSequence=true;
+        //isProcessingEscSequence=true;
-        sequenceStep=0;
+        //sequenceStep=0;
+        sequenceState=State::waitingForSequenceStart;
         break;
     case '\r':
         cursorX = 0;

Console/retro/Console.h

@@ -27,9 +27,6 @@
 #include <vector>
 #include <string>
 
-#define BEL 7
-#define MAX_LEN 250
-
 namespace retro
 {
     class Attributes
@@ -62,8 +59,16 @@ namespace retro
         AttributedChar(char cc, Attributes aa) {c=cc; attrs=aa;}
     };
 
-//    inline bool operator==(const Attributes& lhs, const Attributes& rhs);
+    enum class State
-//    inline bool operator!=(const Attributes& lhs, const Attributes& rhs);
+    {
+        noSequence,
+        waitingForSequenceStart,
+        waitingForControlSequence,
+        waitingForM,
+        waitingForOSCStart,
+        waitingForSemicolon,
+        inWindowName
+    };
 
     class Console
     {
@@ -94,15 +99,14 @@ namespace retro
         bool IsEOF() const { return eof; }
 
     private:
+
+        State sequenceState;
         std::string windowName;
         GrafPtr consolePort = nullptr;
         Rect bounds;
         Attributes currentAttr;
 
         std::vector<AttributedChar> chars, onscreen;
-        bool isProcessingEscSequence;
-        int sequenceStep;
-        bool OSCseq;
 
         short cellSizeX;
         short cellSizeY;
@@ -126,8 +130,7 @@ namespace retro
         void DrawCell(short x, short y, bool erase = true);
         void DrawCells(short x1, short x2, short y, bool erase = true);
         void ScrollUp(short n = 1);
-        void ProcessEscSequence(char c);
+        bool ProcessEscSequence(char c);
-        void ProcessOSCseq(char c);
         void SetAttributes(Attributes aa);
 
         void InvalidateCursor();

Console/retro/ConsoleWindow.cc

@@ -36,10 +36,7 @@ namespace
 ConsoleWindow::ConsoleWindow(Rect r, ConstStr255Param title)
 {
     GrafPtr port;
-        //Retro68 Improved Console
+    win = NewWindow(NULL, &r, "\pRetro68 Console", true, 0, (WindowPtr)-1, true, 0);
-    win = NewWindow(NULL, &r, "\p", true, 0, (WindowPtr)-1, true, 0);
-    setWindowName("Retro68 Console");
-
 
 #if !TARGET_API_MAC_CARBON
     port = win;