Commit 79442545 authored by Douwe Maan's avatar Douwe Maan

Implement diff viewers

parent 64e85fda
......@@ -124,6 +124,30 @@ def editable_diff?(diff_file)
!diff_file.deleted_file? && @merge_request && @merge_request.source_project
end
def diff_render_error_reason(viewer)
case viewer.render_error
when :too_large
"it is too large"
when :server_side_but_stored_externally
case viewer.diff_file.external_storage
when :lfs
'it is stored in LFS'
else
'it is stored externally'
end
end
end
def diff_render_error_options(viewer)
diff_file = viewer.diff_file
options = []
blob_url = namespace_project_blob_path(@project.namespace, @project, tree_join(diff_file.content_sha, diff_file.file_path))
options << link_to('view the blob', blob_url)
options
end
private
def diff_btn(title, name, selected)
......
......@@ -13,14 +13,12 @@ def prepare!
end
def render_error
if blob.stored_externally?
# Files that are not stored in the repository, like LFS files and
# build artifacts, can only be rendered using a client-side viewer,
# since we do not want to read large amounts of data into memory on the
# server side. Client-side viewers use JS and can fetch the file from
# `blob_raw_url` using AJAX.
return :server_side_but_stored_externally
end
# Files that are not stored in the repository, like LFS files and
# build artifacts, can only be rendered using a client-side viewer,
# since we do not want to read large amounts of data into memory on the
# server side. Client-side viewers use JS and can fetch the file from
# `blob_raw_url` using AJAX.
return :server_side_but_stored_externally if blob.stored_externally?
super
end
......
module DiffViewer
class Added < Base
include Simple
include Static
self.partial_name = 'added'
end
end
module DiffViewer
class Base
PARTIAL_PATH_PREFIX = 'projects/diffs/viewers'.freeze
class_attribute :partial_name, :type, :extensions, :file_types, :binary, :switcher_icon, :switcher_title
# These limits relate to the sum of the old and new blob sizes.
# Limits related to the actual size of the diff are enforced in Gitlab::Diff::File.
class_attribute :collapse_limit, :size_limit
delegate :partial_path, :loading_partial_path, :rich?, :simple?, :text?, :binary?, to: :class
attr_reader :diff_file
delegate :project, to: :diff_file
def initialize(diff_file)
@diff_file = diff_file
@initially_binary = diff_file.binary?
end
def self.partial_path
File.join(PARTIAL_PATH_PREFIX, partial_name)
end
def self.rich?
type == :rich
end
def self.simple?
type == :simple
end
def self.binary?
binary
end
def self.text?
!binary?
end
def self.can_render?(diff_file, verify_binary: true)
can_render_blob?(diff_file.old_blob, verify_binary: verify_binary) &&
can_render_blob?(diff_file.new_blob, verify_binary: verify_binary)
end
def self.can_render_blob?(blob, verify_binary: true)
return true if blob.nil?
return false if verify_binary && binary? != blob.binary?
return true if extensions&.include?(blob.extension)
return true if file_types&.include?(blob.file_type)
false
end
def collapsed?
return @collapsed if defined?(@collapsed)
return @collapsed = true if diff_file.collapsed?
@collapsed = !diff_file.expanded? && collapse_limit && diff_file.raw_size > collapse_limit
end
def too_large?
return @too_large if defined?(@too_large)
return @too_large = true if diff_file.too_large?
@too_large = size_limit && diff_file.raw_size > size_limit
end
def binary_detected_after_load?
!@initially_binary && diff_file.binary?
end
# This method is used on the server side to check whether we can attempt to
# render the diff_file at all. Human-readable error messages are found in the
# `BlobHelper#diff_render_error_reason` helper.
def render_error
if too_large?
:too_large
end
end
def prepare!
# To be overridden by subclasses
end
end
end
module DiffViewer
module ClientSide
extend ActiveSupport::Concern
included do
self.collapse_limit = 1.megabyte
self.size_limit = 10.megabytes
end
end
end
module DiffViewer
class Deleted < Base
include Simple
include Static
self.partial_name = 'deleted'
end
end
module DiffViewer
class Image < Base
include Rich
include ClientSide
self.partial_name = 'image'
self.extensions = UploaderHelper::IMAGE_EXT
self.binary = true
self.switcher_icon = 'picture-o'
self.switcher_title = 'image diff'
end
end
module DiffViewer
class ModeChanged < Base
include Simple
include Static
self.partial_name = 'mode_changed'
end
end
module DiffViewer
class NoPreview < Base
include Simple
include Static
self.partial_name = 'no_preview'
self.binary = true
end
end
module DiffViewer
class NotDiffable < Base
include Simple
include Static
self.partial_name = 'not_diffable'
self.binary = true
end
end
module DiffViewer
class Renamed < Base
include Simple
include Static
self.partial_name = 'renamed'
end
end
module DiffViewer
module Rich
extend ActiveSupport::Concern
included do
self.type = :rich
self.switcher_icon = 'file-text-o'
self.switcher_title = 'rendered diff'
end
end
end
module DiffViewer
module ServerSide
extend ActiveSupport::Concern
included do
self.collapse_limit = 1.megabyte
self.size_limit = 5.megabytes
end
def prepare!
diff_file.old_blob&.load_all_data!
diff_file.new_blob&.load_all_data!
end
def render_error
# Files that are not stored in the repository, like LFS files and
# build artifacts, can only be rendered using a client-side viewer,
# since we do not want to read large amounts of data into memory on the
# server side. Client-side viewers use JS and can fetch the file from
# `diff_file_blob_raw_path` and `diff_file_old_blob_raw_path` using AJAX.
return :server_side_but_stored_externally if diff_file.stored_externally?
super
end
end
end
module DiffViewer
module Simple
extend ActiveSupport::Concern
included do
self.type = :simple
self.switcher_icon = 'code'
self.switcher_title = 'source diff'
end
end
end
module DiffViewer
module Static
extend ActiveSupport::Concern
# We can always render a static viewer, even if the diff is too large.
def render_error
nil
end
end
end
module DiffViewer
class Text < Base
include Simple
include ServerSide
self.partial_name = 'text'
self.binary = false
# Since the text diff viewer doesn't render the old and new blobs in full,
# we only need the limits related to the actual size of the diff which are
# already enforced in Gitlab::Diff::File.
self.collapse_limit = nil
self.size_limit = nil
end
end
- diff_file = viewer.diff_file
- url = url_for(params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier))
.nothing-here-block.diff-collapsed{ data: { diff_for_path: url } }
This diff is collapsed.
%a.click-to-expand Click to expand it.
- blob = diff_file.blob
.diff-content
- if diff_file.too_large?
.nothing-here-block This diff could not be displayed because it is too large.
- elsif blob.truncated?
.nothing-here-block The file could not be displayed because it is too large.
- elsif blob.readable_text?
- if !diff_file.diffable?
.nothing-here-block This diff was suppressed by a .gitattributes entry.
- elsif diff_file.collapsed?
- url = url_for(params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier))
.nothing-here-block.diff-collapsed{ data: { diff_for_path: url } }
This diff is collapsed.
%a.click-to-expand
Click to expand it.
- elsif diff_file.diff_lines.length > 0
= render "projects/diffs/viewers/text", diff_file: diff_file
- else
- if diff_file.mode_changed?
.nothing-here-block File mode changed
- elsif diff_file.renamed_file?
.nothing-here-block File moved
- elsif blob.image?
= render "projects/diffs/viewers/image", diff_file: diff_file
- else
.nothing-here-block No preview for this file type
= render 'projects/diffs/viewer', viewer: diff_file.rich_viewer || diff_file.simple_viewer
.nothing-here-block
This #{viewer.switcher_title} could not be displayed because #{diff_render_error_reason(viewer)}.
You can
= diff_render_error_options(viewer).to_sentence(two_words_connector: ' or ', last_word_connector: ', or ').html_safe
instead.
- hidden = local_assigns.fetch(:hidden, false)
.diff-viewer{ data: { type: viewer.type }, class: ('hidden' if hidden) }
- if viewer.render_error
= render 'projects/diffs/render_error', viewer: viewer
- elsif viewer.collapsed?
= render 'projects/diffs/collapsed', viewer: viewer
- else
- viewer.prepare!
-# In the rare case where the first kilobyte of the file looks like text,
-# but the file turns out to actually be binary after loading all data,
-# we fall back on the binary No Preview viewer.
- viewer = DiffViewer::NoPreview.new(viewer.diff_file) if viewer.binary_detected_after_load?
= render viewer.partial_path, viewer: viewer
- diff_file = viewer.diff_file
- blob = diff_file.blob
- old_blob = diff_file.old_blob
- blob_raw_path = diff_file_blob_raw_path(diff_file)
......
- diff_file = viewer.diff_file
.nothing-here-block
File mode changed from #{diff_file.a_mode} to #{diff_file.b_mode}
.nothing-here-block
No preview for this file type
.nothing-here-block
This diff was suppressed by a .gitattributes entry.
- diff_file = viewer.diff_file
- blob = diff_file.blob
- blob.load_all_data!
- total_lines = blob.lines.size
- total_lines -= 1 if total_lines > 0 && blob.lines.last.blank?
- if diff_view == :parallel
......
---
title: Implement diff viewers
merge_request:
author:
......@@ -5,7 +5,20 @@ class File
delegate :new_file?, :deleted_file?, :renamed_file?,
:old_path, :new_path, :a_mode, :b_mode, :mode_changed?,
:submodule?, :too_large?, :collapsed?, to: :diff, prefix: false
:submodule?, :expanded?, :too_large?, :collapsed?, :line_count, to: :diff, prefix: false
# Finding a viewer for a diff file happens based only on extension and whether the
# diff file blobs are binary or text, which means 1 diff file should only be matched by 1 viewer,
# and the order of these viewers doesn't really matter.
#
# However, when the diff file blobs are LFS pointers, we cannot know for sure whether the
# file being pointed to is binary or text. In this case, we match only on
# extension, preferring binary viewers over text ones if both exist, since the
# large files referred to in "Large File Storage" are much more likely to be
# binary than text.
RICH_VIEWERS = [
DiffViewer::Image
].sort_by { |v| v.binary? ? 0 : 1 }.freeze
def initialize(diff, repository:, diff_refs: nil, fallback_diff_refs: nil)
@diff = diff
......@@ -177,6 +190,100 @@ def binary?
def text?
!binary?
end
def external_storage_error?
old_blob&.external_storage_error? || new_blob&.external_storage_error?
end
def stored_externally?
old_blob&.stored_externally? || new_blob&.stored_externally?
end
def external_storage
old_blob&.external_storage || new_blob&.external_storage
end
def content_changed?
old_blob && new_blob && old_blob.id != new_blob.id
end
def different_type?
old_blob && new_blob && old_blob.binary? != new_blob.binary?
end
def size
[old_blob&.size, new_blob&.size].compact.sum
end
def raw_size
[old_blob&.raw_size, new_blob&.raw_size].compact.sum
end
def raw_binary?
old_blob&.raw_binary? || new_blob&.raw_binary?
end
def raw_text?
!raw_binary? && !different_type?
end
def simple_viewer
@simple_viewer ||= simple_viewer_class.new(self)
end
def rich_viewer
return @rich_viewer if defined?(@rich_viewer)
@rich_viewer = rich_viewer_class&.new(self)
end
def rendered_as_text?(ignore_errors: true)
simple_viewer.is_a?(DiffViewer::Text) && (ignore_errors || simple_viewer.render_error.nil?)
end
private
def simple_viewer_class
return DiffViewer::NotDiffable unless diffable?
if content_changed?
if raw_text?
DiffViewer::Text
else
DiffViewer::NoPreview
end
elsif new_file?
if raw_text?
DiffViewer::Text
else
DiffViewer::Added
end
elsif deleted_file?
if raw_text?
DiffViewer::Text
else
DiffViewer::Deleted
end
elsif renamed_file?
DiffViewer::Renamed
elsif mode_changed?
DiffViewer::ModeChanged
end
end
def rich_viewer_class
viewer_class_from(RICH_VIEWERS)
end
def viewer_class_from(classes)
return unless diffable?
return if different_type? || external_storage_error?
return unless new_file? || deleted_file? || content_changed?
verify_binary = !stored_externally?
classes.find { |viewer_class| viewer_class.can_render?(self, verify_binary: verify_binary) }
end
end
end
end
......@@ -17,6 +17,8 @@ class Diff
attr_accessor :expanded
alias_method :expanded?, :expanded
# We need this accessor because of `to_hash` and `init_from_hash`
attr_accessor :too_large
......
......@@ -97,7 +97,7 @@ def each_patch
diff = Gitlab::Git::Diff.new(raw, expanded: expanded)
if !expanded && over_safe_limits?(i)
if !expanded && over_safe_limits?(i) && diff.line_count > 0
diff.collapse!
end
......
......@@ -262,7 +262,7 @@ def file_container(filename)
# Wait for elements to appear to ensure full page reload
expect(page).to have_content('This diff was suppressed by a .gitattributes entry')
expect(page).to have_content('This diff could not be displayed because it is too large.')
expect(page).to have_content('This source diff could not be displayed because it is too large.')
expect(page).to have_content('too_large_image.jpg')
find('.note-textarea')
......
......@@ -17,6 +17,7 @@ def visit_blob(path, anchor: nil, ref: 'master')
it 'displays the blob' do
aggregate_failures do
# shows highlighted Ruby code
expect(page).to have_css(".js-syntax-highlight")
expect(page).to have_content("require 'fileutils'")
# does not show a viewer switcher
......@@ -71,6 +72,7 @@ def visit_blob(path, anchor: nil, ref: 'master')
expect(page).to have_selector('.blob-viewer[data-type="rich"]', visible: false)
# shows highlighted Markdown code
expect(page).to have_css(".js-syntax-highlight")
expect(page).to have_content("[PEP-8](http://www.python.org/dev/peps/pep-0008/)")
# shows an enabled copy button
......@@ -114,6 +116,7 @@ def visit_blob(path, anchor: nil, ref: 'master')
expect(page).to have_selector('#LC1.hll')
# shows highlighted Markdown code
expect(page).to have_css(".js-syntax-highlight")
expect(page).to have_content("[PEP-8](http://www.python.org/dev/peps/pep-0008/)")
# shows an enabled copy button
......
require 'spec_helper'
feature 'Diff file viewer', :js, feature: true do
let(:project) { create(:project, :public, :repository) }
def visit_commit(sha, anchor: nil)
visit namespace_project_commit_path(project.namespace, project, sha, anchor: anchor)
wait_for_requests
end
context 'Ruby file' do
before do
visit_commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d')
end
it 'shows highlighted Ruby code' do
within('.diff-file[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"]') do
expect(page).to have_css(".js-syntax-highlight")
expect(page).to have_content("def popen(cmd, path=nil)")
end
end
end
context 'Ruby file (stored in LFS)' do
before do
project.add_master(project.creator)
@commit_id = Files::CreateService.new(
project,
project.creator,
start_branch: 'master',
branch_name: 'master',
commit_message: "Add Ruby file in LFS",
file_path: 'files/lfs/ruby.rb',
file_content: project.repository.blob_at('master', 'files/lfs/lfs_object.iso').data
).execute[:result]
end
context 'when LFS is enabled on the project' do
before do
allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)