Commit d32be032 authored by Vasilii Iakliushin's avatar Vasilii Iakliushin Committed by Luke Duncalfe

Update syntax highlighting logic

Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/16950
FF issue: https://gitlab.com/gitlab-org/gitlab/-/issues/324159

We apply syntax highlighting to old and new versions of the whole
file. However, we need to highlight only diff lines.

This change should optimize this process. Here we provide only
necessary diff lines to the syntax highlighter.
parent 3d339e40
---
name: diff_line_syntax_highlighting
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56108
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/324159
milestone: '13.10'
type: development
group: group::source code
default_enabled: false
...@@ -86,6 +86,41 @@ module Gitlab ...@@ -86,6 +86,41 @@ module Gitlab
def highlight_line(diff_line) def highlight_line(diff_line)
return unless diff_file && diff_file.diff_refs return unless diff_file && diff_file.diff_refs
if Feature.enabled?(:diff_line_syntax_highlighting, project, default_enabled: :yaml)
diff_line_highlighting(diff_line)
else
blob_highlighting(diff_line)
end
end
def diff_line_highlighting(diff_line)
rich_line = syntax_highlighter(diff_line).highlight(
diff_line.text(prefix: false),
context: { line_number: diff_line.line }
)&.html_safe
# Only update text if line is found. This will prevent
# issues with submodules given the line only exists in diff content.
if rich_line
line_prefix = diff_line.text =~ /\A(.)/ ? Regexp.last_match(1) : ' '
rich_line.prepend(line_prefix).concat("\n")
end
end
def syntax_highlighter(diff_line)
path = diff_line.removed? ? diff_file.old_path : diff_file.new_path
@syntax_highlighter ||= {}
@syntax_highlighter[path] ||= Gitlab::Highlight.new(
path,
@raw_lines,
language: repository&.gitattribute(path, 'gitlab-language')
)
end
# Deprecated: https://gitlab.com/gitlab-org/gitlab/-/issues/324159
# ------------------------------------------------------------------------
def blob_highlighting(diff_line)
rich_line = rich_line =
if diff_line.unchanged? || diff_line.added? if diff_line.unchanged? || diff_line.added?
new_lines[diff_line.new_pos - 1]&.html_safe new_lines[diff_line.new_pos - 1]&.html_safe
...@@ -102,6 +137,7 @@ module Gitlab ...@@ -102,6 +137,7 @@ module Gitlab
end end
# Deprecated: https://gitlab.com/gitlab-org/gitlab/-/issues/324638 # Deprecated: https://gitlab.com/gitlab-org/gitlab/-/issues/324638
# ------------------------------------------------------------------------
def inline_diffs def inline_diffs
@inline_diffs ||= InlineDiff.for_lines(@raw_lines) @inline_diffs ||= InlineDiff.for_lines(@raw_lines)
end end
......
...@@ -71,10 +71,12 @@ module Gitlab ...@@ -71,10 +71,12 @@ module Gitlab
strong_memoize(:redis_key) do strong_memoize(:redis_key) do
[ [
'highlighted-diff-files', 'highlighted-diff-files',
diffable.cache_key, VERSION, diffable.cache_key,
VERSION,
diff_options, diff_options,
Feature.enabled?(:introduce_marker_ranges, diffable.project, default_enabled: :yaml), Feature.enabled?(:introduce_marker_ranges, diffable.project, default_enabled: :yaml),
Feature.enabled?(:use_marker_ranges, diffable.project, default_enabled: :yaml) Feature.enabled?(:use_marker_ranges, diffable.project, default_enabled: :yaml),
Feature.enabled?(:diff_line_syntax_highlighting, diffable.project, default_enabled: :yaml)
].join(":") ].join(":")
end end
end end
......
...@@ -9,8 +9,8 @@ module Gitlab ...@@ -9,8 +9,8 @@ module Gitlab
SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze
attr_reader :line_code, :marker_ranges attr_reader :line_code, :marker_ranges
attr_writer :rich_text attr_writer :text, :rich_text
attr_accessor :text, :index, :type, :old_pos, :new_pos attr_accessor :index, :type, :old_pos, :new_pos
def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil) def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil)
@text, @type, @index = text, type, index @text, @type, @index = text, type, index
...@@ -54,6 +54,12 @@ module Gitlab ...@@ -54,6 +54,12 @@ module Gitlab
@marker_ranges = marker_ranges @marker_ranges = marker_ranges
end end
def text(prefix: true)
return @text if prefix
@text&.slice(1..).to_s
end
def old_line def old_line
old_pos unless added? || meta? old_pos unless added? || meta?
end end
......
...@@ -20,7 +20,9 @@ module Gitlab ...@@ -20,7 +20,9 @@ module Gitlab
@blob_content = blob_content @blob_content = blob_content
end end
def highlight(text, continue: true, plain: false) def highlight(text, continue: false, plain: false, context: {})
@context = context
plain ||= text.length > MAXIMUM_TEXT_HIGHLIGHT_SIZE plain ||= text.length > MAXIMUM_TEXT_HIGHLIGHT_SIZE
highlighted_text = highlight_text(text, continue: continue, plain: plain) highlighted_text = highlight_text(text, continue: continue, plain: plain)
...@@ -38,6 +40,8 @@ module Gitlab ...@@ -38,6 +40,8 @@ module Gitlab
private private
attr_reader :context
def custom_language def custom_language
return unless @language return unless @language
...@@ -53,13 +57,13 @@ module Gitlab ...@@ -53,13 +57,13 @@ module Gitlab
end end
def highlight_plain(text) def highlight_plain(text)
@formatter.format(Rouge::Lexers::PlainText.lex(text)).html_safe @formatter.format(Rouge::Lexers::PlainText.lex(text), context).html_safe
end end
def highlight_rich(text, continue: true) def highlight_rich(text, continue: true)
tag = lexer.tag tag = lexer.tag
tokens = lexer.lex(text, continue: continue) tokens = lexer.lex(text, continue: continue)
Timeout.timeout(timeout_time) { @formatter.format(tokens, tag: tag).html_safe } Timeout.timeout(timeout_time) { @formatter.format(tokens, context.merge(tag: tag)).html_safe }
rescue Timeout::Error => e rescue Timeout::Error => e
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
highlight_plain(text) highlight_plain(text)
......
...@@ -8,9 +8,10 @@ module Rouge ...@@ -8,9 +8,10 @@ module Rouge
# Creates a new <tt>Rouge::Formatter::HTMLGitlab</tt> instance. # Creates a new <tt>Rouge::Formatter::HTMLGitlab</tt> instance.
# #
# [+tag+] The tag (language) of the lexer used to generate the formatted tokens # [+tag+] The tag (language) of the lexer used to generate the formatted tokens
# [+line_number+] The line number used to populate line IDs
def initialize(options = {}) def initialize(options = {})
@line_number = 1
@tag = options[:tag] @tag = options[:tag]
@line_number = options[:line_number] || 1
end end
def stream(tokens) def stream(tokens)
......
...@@ -238,26 +238,36 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do ...@@ -238,26 +238,36 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
subject { cache.key } subject { cache.key }
it 'returns cache key' do it 'returns cache key' do
is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:true:true") is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:true:true:true")
end end
context 'when feature flag is disabled' do context 'when the `introduce_marker_ranges` feature flag is disabled' do
before do before do
stub_feature_flags(introduce_marker_ranges: false) stub_feature_flags(introduce_marker_ranges: false)
end end
it 'returns the original version of the cache' do it 'returns the original version of the cache' do
is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:false:true") is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:false:true:true")
end end
end end
context 'when use marker ranges feature flag is disabled' do context 'when the `use_marker_ranges` feature flag is disabled' do
before do before do
stub_feature_flags(use_marker_ranges: false) stub_feature_flags(use_marker_ranges: false)
end end
it 'returns the original version of the cache' do it 'returns the original version of the cache' do
is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:true:false") is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:true:false:true")
end
end
context 'when the `diff_line_syntax_highlighting` feature flag is disabled' do
before do
stub_feature_flags(diff_line_syntax_highlighting: false)
end
it 'returns the original version of the cache' do
is_expected.to eq("highlighted-diff-files:#{cache.diffable.cache_key}:2:#{cache.diff_options}:true:true:false")
end end
end end
end end
......
...@@ -45,6 +45,29 @@ RSpec.describe Gitlab::Diff::Line do ...@@ -45,6 +45,29 @@ RSpec.describe Gitlab::Diff::Line do
end end
end end
describe '#text' do
let(:line) { described_class.new(raw_diff, 'new', 0, 0, 0) }
let(:raw_diff) { '+Hello' }
it 'returns raw diff text' do
expect(line.text).to eq('+Hello')
end
context 'when prefix is disabled' do
it 'returns raw diff text without prefix' do
expect(line.text(prefix: false)).to eq('Hello')
end
context 'when diff is empty' do
let(:raw_diff) { '' }
it 'returns an empty raw diff' do
expect(line.text(prefix: false)).to eq('')
end
end
end
end
context "when setting rich text" do context "when setting rich text" do
it 'escapes any HTML special characters in the diff chunk header' do it 'escapes any HTML special characters in the diff chunk header' do
subject = described_class.new("<input>", "", 0, 0, 0) subject = described_class.new("<input>", "", 0, 0, 0)
......
...@@ -79,6 +79,21 @@ RSpec.describe Gitlab::Highlight do ...@@ -79,6 +79,21 @@ RSpec.describe Gitlab::Highlight do
expect(result).to eq(expected) expect(result).to eq(expected)
end end
context 'when start line number is set' do
let(:expected) do
%q(<span id="LC10" class="line" lang="diff"><span class="gi">+aaa</span></span>
<span id="LC11" class="line" lang="diff"><span class="gi">+bbb</span></span>
<span id="LC12" class="line" lang="diff"><span class="gd">- ccc</span></span>
<span id="LC13" class="line" lang="diff"> ddd</span>)
end
it 'highlights each line properly' do
result = described_class.new(file_name, content).highlight(content, context: { line_number: 10 })
expect(result).to eq(expected)
end
end
end end
describe 'with CRLF' do describe 'with CRLF' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Rouge::Formatters::HTMLGitlab do
describe '#format' do
subject { described_class.format(tokens, options) }
let(:lang) { 'ruby' }
let(:lexer) { Rouge::Lexer.find_fancy(lang) }
let(:tokens) { lexer.lex("def hello", continue: false) }
let(:options) { { tag: lang } }
it 'returns highlighted ruby code' do
code = %q{<span id="LC1" class="line" lang="ruby"><span class="k">def</span> <span class="nf">hello</span></span>}
is_expected.to eq(code)
end
context 'when options are empty' do
let(:options) { {} }
it 'returns highlighted code without language' do
code = %q{<span id="LC1" class="line" lang=""><span class="k">def</span> <span class="nf">hello</span></span>}
is_expected.to eq(code)
end
end
context 'when line number is provided' do
let(:options) { { tag: lang, line_number: 10 } }
it 'returns highlighted ruby code with correct line number' do
code = %q{<span id="LC10" class="line" lang="ruby"><span class="k">def</span> <span class="nf">hello</span></span>}
is_expected.to eq(code)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment