Support \r as line break for job logs

This bugfix is usefult to translate terminal behavior
where \r would refresh the current line or for Mac's
line break.

changelogs/unreleased/job-log-support-mac-line-break.yml

+---
+title: Let ANSI \r code replace the current job log line
+merge_request: 18933
+author:
+type: fixed

lib/gitlab/ci/ansi2json/converter.rb

@@ -22,11 +22,11 @@ module Gitlab
 
           start_offset = @state.offset
 
-          @state.set_current_line!(style: Style.new(@state.inherited_style))
+          @state.new_line!(
+            style: Style.new(@state.inherited_style))
 
           stream.each_line do |line|
-            s = StringScanner.new(line)
-            convert_line(s)
+            consume_line(line)
           end
 
           # This must be assigned before flushing the current line
@@ -52,26 +52,43 @@ module Gitlab
 
         private
 
-        def convert_line(scanner)
-          until scanner.eos?
-
-            if scanner.scan(Gitlab::Regex.build_trace_section_regex)
-              handle_section(scanner)
-            elsif scanner.scan(/\e([@-_])(.*?)([@-~])/)
-              handle_sequence(scanner)
-            elsif scanner.scan(/\e(([@-_])(.*?)?)?$/)
-              break
-            elsif scanner.scan(/</)
-              @state.current_line << '&lt;'
-            elsif scanner.scan(/\r?\n/)
-              # we advance the offset of the next current line
-              # so it does not start from \n
-              flush_current_line(advance_offset: scanner.matched_size)
-            else
-              @state.current_line << scanner.scan(/./m)
-            end
-
-            @state.offset += scanner.matched_size
+        def consume_line(line)
+          scanner = StringScanner.new(line)
+
+          consume_token(scanner) until scanner.eos?
+        end
+
+        def consume_token(scanner)
+          if scan_token(scanner, Gitlab::Regex.build_trace_section_regex, consume: false)
+            handle_section(scanner)
+          elsif scan_token(scanner, /\e([@-_])(.*?)([@-~])/)
+            handle_sequence(scanner)
+          elsif scan_token(scanner, /\e(([@-_])(.*?)?)?$/)
+            # stop scanning
+            scanner.terminate
+          elsif scan_token(scanner, /</)
+            @state.current_line << '&lt;'
+          elsif scan_token(scanner, /\r?\n/)
+            flush_current_line
+          elsif scan_token(scanner, /\r/)
+            # drop last line
+            @state.current_line.clear!
+          elsif scan_token(scanner, /.[^\e<\r\ns]*/m)
+            # this is a join from all previous tokens and first letters
+            # it always matches at least one character `.`
+            # it matches everything that is not start of:
+            # `\e`, `<`, `\r`, `\n`, `s` (for section_start)
+            @state.current_line << scanner[0]
+          else
+            raise 'invalid parser state'
+          end
+        end
+
+        def scan_token(scanner, match, consume: true)
+          scanner.scan(match).tap do |result|
+            # we need to move offset as soon
+            # as we match the token
+            @state.offset += scanner.matched_size if consume && result
           end
         end
 
@@ -96,32 +113,50 @@ module Gitlab
           section_name = sanitize_section_name(section)
 
           if action == "start"
-            handle_section_start(section_name, timestamp)
+            handle_section_start(scanner, section_name, timestamp)
           elsif action == "end"
-            handle_section_end(section_name, timestamp)
+            handle_section_end(scanner, section_name, timestamp)
+          else
+            raise 'unsupported action'
           end
         end
 
-        def handle_section_start(section, timestamp)
-          flush_current_line unless @state.current_line.empty?
+        def handle_section_start(scanner, section, timestamp)
+          # We make a new line for new section
+          flush_current_line
+
           @state.open_section(section, timestamp)
+
+          # we need to consume match after handling
+          # the open of section, as we want the section
+          # marker to be refresh on incremental update
+          @state.offset += scanner.matched_size
         end
 
-        def handle_section_end(section, timestamp)
+        def handle_section_end(scanner, section, timestamp)
           return unless @state.section_open?(section)
 
-          flush_current_line unless @state.current_line.empty?
+          # We flush the content to make the end
+          # of section to be a new line
+          flush_current_line
+
           @state.close_section(section, timestamp)
 
-          # ensure that section end is detached from the last
-          # line in the section
+          # we need to consume match before handling
+          # as we want the section close marker
+          # not to be refreshed on incremental update
+          @state.offset += scanner.matched_size
+
+          # this flushes an empty line with `section_duration`
           flush_current_line
         end
 
-        def flush_current_line(advance_offset: 0)
-          @lines << @state.current_line.to_h
+        def flush_current_line
+          unless @state.current_line.empty?
+            @lines << @state.current_line.to_h
+          end
 
-          @state.set_current_line!(advance_offset: advance_offset)
+          @state.new_line!
         end
 
         def sanitize_section_name(section)

lib/gitlab/ci/ansi2json/line.rb

@@ -47,12 +47,17 @@ module Gitlab
           @current_segment.text << data
         end
 
+        def clear!
+          @segments.clear
+          @current_segment = Segment.new(style: style)
+        end
+
         def style
           @current_segment.style
         end
 
         def empty?
-          @segments.empty? && @current_segment.empty?
+          @segments.empty? && @current_segment.empty? && @section_duration.nil?
         end
 
         def update_style(ansi_commands)

lib/gitlab/ci/ansi2json/state.rb

@@ -46,9 +46,9 @@ module Gitlab
           @open_sections.key?(section)
         end
 
-        def set_current_line!(style: nil, advance_offset: 0)
+        def new_line!(style: nil)
           new_line = Line.new(
-            offset: @offset + advance_offset,
+            offset: @offset,
             style: style || @current_line.style,
             sections: @open_sections.keys
           )

spec/lib/gitlab/ci/ansi2json_spec.rb

@@ -12,11 +12,26 @@ describe Gitlab::Ci::Ansi2json do
       ])
     end
 
-    it 'adds new line in a separate element' do
-      expect(convert_json("Hello\nworld")).to eq([
-        { offset: 0, content: [{ text: 'Hello' }] },
-        { offset: 6, content: [{ text: 'world' }] }
-      ])
+    context 'new lines' do
+      it 'adds new line when encountering \n' do
+        expect(convert_json("Hello\nworld")).to eq([
+          { offset: 0, content: [{ text: 'Hello' }] },
+          { offset: 6, content: [{ text: 'world' }] }
+        ])
+      end
+
+      it 'adds new line when encountering \r\n' do
+        expect(convert_json("Hello\r\nworld")).to eq([
+          { offset: 0, content: [{ text: 'Hello' }] },
+          { offset: 7, content: [{ text: 'world' }] }
+        ])
+      end
+
+      it 'replace the current line when encountering \r' do
+        expect(convert_json("Hello\rworld")).to eq([
+          { offset: 0, content: [{ text: 'world' }] }
+        ])
+      end
     end
 
     it 'recognizes color changing ANSI sequences' do
@@ -113,10 +128,6 @@ describe Gitlab::Ci::Ansi2json do
             content: [],
             section_duration: '01:03',
             section: 'prepare-script'
-          },
-          {
-            offset: 63,
-            content: []
           }
         ])
       end
@@ -134,10 +145,6 @@ describe Gitlab::Ci::Ansi2json do
             content: [],
             section: 'prepare-script',
             section_duration: '01:03'
-          },
-          {
-            offset: 56,
-            content: []
           }
         ])
       end
@@ -157,7 +164,7 @@ describe Gitlab::Ci::Ansi2json do
             section_duration: '01:03'
           },
           {
-            offset: 49,
+            offset: 91,
             content: [{ text: 'world' }]
           }
         ])
@@ -198,7 +205,7 @@ describe Gitlab::Ci::Ansi2json do
           expect(convert_json("#{section_start}hello")).to eq([
             {
               offset: 0,
-              content: [{ text: "#{section_start.gsub("\033[0K", '')}hello" }]
+              content: [{ text: 'hello' }]
             }
           ])
         end
@@ -211,7 +218,7 @@ describe Gitlab::Ci::Ansi2json do
           expect(convert_json("#{section_start}hello")).to eq([
             {
               offset: 0,
-              content: [{ text: "#{section_start.gsub("\033[0K", '').gsub('<', '&lt;')}hello" }]
+              content: [{ text: 'hello' }]
             }
           ])
         end
@@ -231,10 +238,6 @@ describe Gitlab::Ci::Ansi2json do
             content: [],
             section: 'prepare-script',
             section_duration: '01:03'
-          },
-          {
-            offset: 95,
-            content: []
           }
         ])
       end
@@ -274,7 +277,7 @@ describe Gitlab::Ci::Ansi2json do
                 section_duration: '00:02'
               },
               {
-                offset: 106,
+                offset: 155,
                 content: [{ text: 'baz' }],
                 section: 'prepare-script'
               },
@@ -285,7 +288,7 @@ describe Gitlab::Ci::Ansi2json do
                 section_duration: '01:03'
               },
               {
-                offset: 158,
+                offset: 200,
                 content: [{ text: 'world' }]
               }
             ])
@@ -318,14 +321,10 @@ describe Gitlab::Ci::Ansi2json do
                 section_duration: '00:02'
               },
               {
-                offset: 115,
+                offset: 164,
                 content: [],
                 section: 'prepare-script',
                 section_duration: '01:03'
-              },
-              {
-                offset: 164,
-                content: []
               }
             ])
         end
@@ -380,7 +379,7 @@ describe Gitlab::Ci::Ansi2json do
           ]
         end
 
-        it 'returns the full line' do
+        it 'returns the line since last partially processed line' do
           expect(pass2.lines).to eq(lines)
           expect(pass2.append).to be_truthy
         end
@@ -399,7 +398,7 @@ describe Gitlab::Ci::Ansi2json do
           ]
         end
 
-        it 'returns the full line' do
+        it 'returns the line since last partially processed line' do
           expect(pass2.lines).to eq(lines)
           expect(pass2.append).to be_falsey
         end
@@ -416,7 +415,7 @@ describe Gitlab::Ci::Ansi2json do
           ]
         end
 
-        it 'returns the full line' do
+        it 'returns a blank line and the next line' do
           expect(pass2.lines).to eq(lines)
           expect(pass2.append).to be_falsey
         end
@@ -502,10 +501,6 @@ describe Gitlab::Ci::Ansi2json do
                 content: [],
                 section: 'prepare-script',
                 section_duration: '01:03'
-              },
-              {
-                offset: 77,
-                content: []
               }
             ]
           end