Merge pull request #13684 from eduardoj/feature/diff_parser

Implement parser for request diffs

src/api/app/assets/stylesheets/webui/diff.scss

@@ -9,3 +9,68 @@
   right: 0.55rem;
   z-index: 4;
 }
+
+.pre.scroll {
+  overflow: auto;
+  border-bottom-left-radius: calc(#{$card-border-radius} - #{$card-border-width});
+  border-bottom-right-radius: calc(#{$card-border-radius} - #{$card-border-width});
+  .diff {
+    min-width: 100%;
+    width: max-content;
+    font-family: $font-family-monospace;
+    .line {
+      display: flex;
+      .character {
+        width: 1ch;
+        white-space: pre;
+        padding-left: 1rem
+      }
+      a {
+        flex-shrink: 0;
+        position: sticky;
+        left: 0;
+        .number {
+          color: $text-muted;
+          width: 4rem;
+          text-align: right;
+          padding: 0 1rem;
+          border-right: $card-border-width solid $card-border-color;
+          user-select: none;
+          background-color: $card-bg;
+        }
+      }
+      &.added, &.added .number {
+        background-color: rgba(mix($card-bg, rgba($success, 0.2)), 1)
+      }
+      &.removed, &.removed .number {
+        background-color: rgba(mix($card-bg, rgba($danger, 0.2)), 1)
+      }
+      &.range, &.comment {
+        border-bottom: $card-border-width solid $card-border-color;
+        border-top: $card-border-width solid $card-border-color;
+        .offset {
+          padding-left: calc(8rem - #{$card-border-width});
+          position: sticky;
+          left: 0;
+          .text {
+            padding-left: 1rem;
+            border-left: $card-border-width solid $card-border-color;
+          }
+        }
+      }
+      .content {
+        margin-left: 1rem;
+        white-space: pre;
+      }
+      &:target {
+        .number {
+          background-color: rgba(mix($card-bg, rgba($warning, 0.5)), 1)
+        }
+        .value {
+          background-color: rgba($warning, 0.1);
+          width: 100%
+        }
+      }
+    }
+  }
+}

src/api/app/components/sourcediff_component.rb

@@ -11,6 +11,8 @@ def initialize(bs_request:, action:, index:, refresh:)
     @action = action
     @index = index
     @refresh = refresh
+
+    ActionSourcediffParser.new(action_sourcediff: action[:sourcediff]).call
   end
 
   def file_view_path(filename, sourcediff)

src/api/app/services/action_sourcediff_parser.rb

@@ -0,0 +1,15 @@
+class ActionSourcediffParser
+  def initialize(action_sourcediff:)
+    @action_sourcediff = action_sourcediff
+  end
+
+  def call
+    @action_sourcediff&.each do |sourcediff|
+      sourcediff['filenames']&.each do |filename|
+        content = sourcediff['files'][filename].dig('diff', '_content')
+
+        sourcediff['files'][filename]['diff']['parsed_lines'] = DiffParser.new(content: content).call
+      end
+    end
+  end
+end

src/api/app/services/diff_parser.rb

@@ -0,0 +1,62 @@
+class DiffParser
+  def initialize(content: [])
+    @content = content
+    @original_index = 0
+    @changed_index = 0
+    @last_original_index = 0
+    @last_changed_index = 0
+    @lines = []
+  end
+
+  def call
+    @content.each_line.with_index do |line, line_index|
+      @state = parse_line(line)
+
+      @lines << create_line(line, line_index)
+
+      @last_original_index = @original_index
+      @last_changed_index = @changed_index
+    end
+
+    @lines
+  end
+
+  private
+
+  def parse_line(line)
+    case line
+    when /^ /
+      @original_index += 1
+      @changed_index += 1
+      'unchanged'
+    when /^@@ -(?<original_index>\d+).+\+(?<changed_index>\d+),/
+      @original_index = Regexp.last_match[:original_index].to_i - 1
+      @changed_index = Regexp.last_match[:changed_index].to_i - 1
+      'range'
+    when /^-/
+      @original_index += 1
+      'removed'
+    when /^[+]/
+      @changed_index += 1
+      'added'
+    else
+      'comment'
+    end
+  end
+
+  def create_line(line, line_index)
+    DiffParser::Line.new(content: line, state: @state, index: line_index + 1, original_index: final_original_index, changed_index: final_changed_index)
+  end
+
+  def final_original_index
+    return if ['comment', 'range'].include?(@state)
+
+    @original_index unless @last_original_index == @original_index
+  end
+
+  def final_changed_index
+    return if ['comment', 'range'].include?(@state)
+
+    @changed_index unless @last_changed_index == @changed_index
+  end
+end

src/api/app/services/diff_parser/line.rb

@@ -0,0 +1,17 @@
+class DiffParser
+  class Line
+    attr_reader :content, :state, :index, :original_index, :changed_index
+
+    def initialize(content:, state:, index:, original_index:, changed_index:)
+      @content = content
+      @state = state
+      @index = index
+      @original_index = original_index
+      @changed_index = changed_index
+    end
+
+    def ==(other)
+      content == other.content && state == other.state && index == other.index && original_index == other.original_index && changed_index == other.changed_index
+    end
+  end
+end

src/api/app/views/webui/package/_revision_diff_detail.html.haml

@@ -8,7 +8,32 @@
         .diff-card-header
           = calculate_filename(filename, file)
           %span.badge{ class: "badge-#{file['state']}" }= file['state'].capitalize
-      = render_diff(diff_content, file_index: index)
+      -# TODO: Remove this `if` condition, and the `else` clause once request_show_redesign is rolled out
+      - if Flipper.enabled?(:request_show_redesign, User.session)
+        .pre.scroll
+          .diff>
+            - diff_parsed_lines = file.dig('diff', 'parsed_lines')
+            - diff_parsed_lines&.each do |line|
+              - id = "diff_#{index}_n#{line.index}"
+              %div{ class: "line #{line.state}", id: }>
+                - case line.state
+                - when 'comment', 'range'
+                  .offset
+                    .text-muted.text
+                      = line.content
+                - else
+                  = link_to "##{id}", class: 'd-flex' do
+                    .number<
+                      = line.original_index
+                    .number<
+                      = line.changed_index
+                  .value<
+                    %span.character><
+                      = line.content[0]
+                    %span.content><
+                      = line.content[1..]
+      - else
+        = render_diff(diff_content, file_index: index)
   - else
     .card{ id: "revision_details_#{index}" }
       .card-header

src/api/spec/services/diff_parser_spec.rb

@@ -0,0 +1,50 @@
+require 'rails_helper'
+
+RSpec.describe DiffParser, type: :service do
+  let(:content) { Rails.root.join("spec/support/files/#{file}").expand_path }
+  let(:parser) { described_class.new(content: content) }
+
+  let(:result) { result_array.map { |line| DiffParser::Line.new(content: line[0], state: line[1], index: line[2], original_index: line[3], changed_index: line[4]) } }
+
+  subject { parser.call }
+
+  describe '#call' do
+    context 'empty diff' do
+      let(:content) { '' }
+
+      let(:result_array) { [] }
+
+      it { expect(subject).to eq(result) }
+    end
+
+    context 'simple diff' do
+      let(:file) { 'diff_simple.diff' }
+
+      let(:result_array) do
+        [
+          ["@@ -1,1 +1,1 @@\n", 'range', 1, nil, nil],
+          ["-a\n", 'removed', 2, 1, nil],
+          ["+b\n", 'added', 3, nil, 1]
+        ]
+      end
+
+      it 'parses correctly' do expect(subject).to eq(result) end
+    end
+
+    context 'diff with no newline comments' do
+      let(:file) { 'diff_with_no_newline_comments.diff' }
+
+      let(:result_array) do
+        [
+          ["@@ -1,1 +1,1 @@\n", 'range', 1, nil, nil],
+          ["-a\n", 'removed', 2, 1, nil],
+          ["\\ No newline at end of file\n", 'comment', 3, nil, nil],
+          ["+b\n", 'added', 4, nil, 1],
+          ["\\ No newline at end of file\n", 'comment', 5, nil, nil]
+        ]
+      end
+
+      it 'parses correctly' do expect(subject).to eq(result) end
+    end
+  end
+end

src/api/spec/support/files/diff_simple.diff

@@ -0,0 +1,3 @@
+@@ -1,1 +1,1 @@
+-a
++b

src/api/spec/support/files/diff_with_no_newline_comments.diff

@@ -0,0 +1,5 @@
+@@ -1,1 +1,1 @@
+-a
+\ No newline at end of file
++b
+\ No newline at end of file