Commit c51ac0484f37b65b31cd83b816b151fc7d50cf05
Exists in
spb-stable
and in
2 other branches
Merge pull request #5962 from skv-headless/editing-preview
editing preview
Showing
15 changed files
with
146 additions
and
17 deletions
Show diff stats
Gemfile
| @@ -83,6 +83,9 @@ gem "seed-fu" | @@ -83,6 +83,9 @@ gem "seed-fu" | ||
| 83 | gem "redcarpet", "~> 2.2.2" | 83 | gem "redcarpet", "~> 2.2.2" |
| 84 | gem "github-markup" | 84 | gem "github-markup" |
| 85 | 85 | ||
| 86 | +# Diffs | ||
| 87 | +gem 'diffy', '~> 3.0.3' | ||
| 88 | + | ||
| 86 | # Asciidoc to HTML | 89 | # Asciidoc to HTML |
| 87 | gem "asciidoctor" | 90 | gem "asciidoctor" |
| 88 | 91 |
Gemfile.lock
| @@ -101,6 +101,7 @@ GEM | @@ -101,6 +101,7 @@ GEM | ||
| 101 | devise-async (0.8.0) | 101 | devise-async (0.8.0) |
| 102 | devise (>= 2.2, < 3.2) | 102 | devise (>= 2.2, < 3.2) |
| 103 | diff-lcs (1.2.5) | 103 | diff-lcs (1.2.5) |
| 104 | + diffy (3.0.3) | ||
| 104 | docile (1.1.1) | 105 | docile (1.1.1) |
| 105 | dotenv (0.9.0) | 106 | dotenv (0.9.0) |
| 106 | email_spec (1.5.0) | 107 | email_spec (1.5.0) |
| @@ -577,6 +578,7 @@ DEPENDENCIES | @@ -577,6 +578,7 @@ DEPENDENCIES | ||
| 577 | default_value_for (~> 3.0.0) | 578 | default_value_for (~> 3.0.0) |
| 578 | devise (= 3.0.4) | 579 | devise (= 3.0.4) |
| 579 | devise-async (= 0.8.0) | 580 | devise-async (= 0.8.0) |
| 581 | + diffy (~> 3.0.3) | ||
| 580 | email_spec | 582 | email_spec |
| 581 | email_validator (~> 1.4.0) | 583 | email_validator (~> 1.4.0) |
| 582 | enumerize | 584 | enumerize |
app/assets/stylesheets/generic/files.scss
app/controllers/projects/edit_tree_controller.rb
| @@ -26,6 +26,18 @@ class Projects::EditTreeController < Projects::BaseTreeController | @@ -26,6 +26,18 @@ class Projects::EditTreeController < Projects::BaseTreeController | ||
| 26 | end | 26 | end |
| 27 | end | 27 | end |
| 28 | 28 | ||
| 29 | + def preview | ||
| 30 | + @content = params[:content] | ||
| 31 | + #FIXME workaround https://github.com/gitlabhq/gitlabhq/issues/5936 | ||
| 32 | + @content += "\n" if @blob.data.end_with?("\n") | ||
| 33 | + | ||
| 34 | + diffy = Diffy::Diff.new(@blob.data, @content, diff: '-U 3', | ||
| 35 | + include_diff_info: true) | ||
| 36 | + @diff = Gitlab::DiffParser.new(diffy.diff.scan(/.*\n/)) | ||
| 37 | + | ||
| 38 | + render layout: false | ||
| 39 | + end | ||
| 40 | + | ||
| 29 | private | 41 | private |
| 30 | 42 | ||
| 31 | def blob | 43 | def blob |
app/helpers/commits_helper.rb
| @@ -16,9 +16,10 @@ module CommitsHelper | @@ -16,9 +16,10 @@ module CommitsHelper | ||
| 16 | end | 16 | end |
| 17 | 17 | ||
| 18 | def each_diff_line(diff, index) | 18 | def each_diff_line(diff, index) |
| 19 | - Gitlab::DiffParser.new(diff).each do |full_line, type, line_code, line_new, line_old| | ||
| 20 | - yield(full_line, type, line_code, line_new, line_old) | ||
| 21 | - end | 19 | + Gitlab::DiffParser.new(diff.diff.lines.to_a, diff.new_path) |
| 20 | + .each do |full_line, type, line_code, line_new, line_old| | ||
| 21 | + yield(full_line, type, line_code, line_new, line_old) | ||
| 22 | + end | ||
| 22 | end | 23 | end |
| 23 | 24 | ||
| 24 | def each_diff_line_near(diff, index, expected_line_code) | 25 | def each_diff_line_near(diff, index, expected_line_code) |
app/helpers/tree_helper.rb
| @@ -91,4 +91,12 @@ module TreeHelper | @@ -91,4 +91,12 @@ module TreeHelper | ||
| 91 | def leave_edit_message | 91 | def leave_edit_message |
| 92 | "Leave edit mode?\nAll unsaved changes will be lost." | 92 | "Leave edit mode?\nAll unsaved changes will be lost." |
| 93 | end | 93 | end |
| 94 | + | ||
| 95 | + def editing_preview_title(filename) | ||
| 96 | + if gitlab_markdown?(filename) || markup?(filename) | ||
| 97 | + 'Preview' | ||
| 98 | + else | ||
| 99 | + 'Diff' | ||
| 100 | + end | ||
| 101 | + end | ||
| 94 | end | 102 | end |
app/models/note.rb
| @@ -184,9 +184,10 @@ class Note < ActiveRecord::Base | @@ -184,9 +184,10 @@ class Note < ActiveRecord::Base | ||
| 184 | return @diff_line if @diff_line | 184 | return @diff_line if @diff_line |
| 185 | 185 | ||
| 186 | if diff | 186 | if diff |
| 187 | - Gitlab::DiffParser.new(diff).each do |full_line, type, line_code, line_new, line_old| | ||
| 188 | - @diff_line = full_line if line_code == self.line_code | ||
| 189 | - end | 187 | + Gitlab::DiffParser.new(diff.diff.lines.to_a, diff.new_path) |
| 188 | + .each do |full_line, type, line_code, line_new, line_old| | ||
| 189 | + @diff_line = full_line if line_code == self.line_code | ||
| 190 | + end | ||
| 190 | end | 191 | end |
| 191 | 192 | ||
| 192 | @diff_line | 193 | @diff_line |
| @@ -0,0 +1,13 @@ | @@ -0,0 +1,13 @@ | ||
| 1 | +%table.text-file | ||
| 2 | + - each_diff_line(diff, 1) do |line, type, line_code, line_new, line_old, raw_line| | ||
| 3 | + %tr.line_holder{ id: line_code, class: "#{type}" } | ||
| 4 | + - if type == "match" | ||
| 5 | + %td.old_line= "..." | ||
| 6 | + %td.new_line= "..." | ||
| 7 | + %td.line_content.matched= line | ||
| 8 | + - else | ||
| 9 | + %td.old_line | ||
| 10 | + = link_to raw(type == "new" ? " " : line_old), "##{line_code}", id: line_code | ||
| 11 | + %td.new_line= link_to raw(type == "old" ? " " : line_new) , "##{line_code}", id: line_code | ||
| 12 | + %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line) | ||
| 13 | + |
| @@ -0,0 +1,26 @@ | @@ -0,0 +1,26 @@ | ||
| 1 | +.diff-file | ||
| 2 | + .diff-content | ||
| 3 | + - if gitlab_markdown?(@blob.name) | ||
| 4 | + .file-content.wiki | ||
| 5 | + = preserve do | ||
| 6 | + = markdown(@content) | ||
| 7 | + - elsif markup?(@blob.name) | ||
| 8 | + .file-content.wiki | ||
| 9 | + = raw GitHub::Markup.render(@blob.name, @content) | ||
| 10 | + - else | ||
| 11 | + .file-content.code | ||
| 12 | + - unless @diff.empty? | ||
| 13 | + %table.text-file | ||
| 14 | + - @diff.each do |line, type, line_code, line_new, line_old, raw_line| | ||
| 15 | + %tr.line_holder{ id: line_code, class: "#{type}" } | ||
| 16 | + - if type == "match" | ||
| 17 | + %td.old_line= "..." | ||
| 18 | + %td.new_line= "..." | ||
| 19 | + %td.line_content.matched= line | ||
| 20 | + - else | ||
| 21 | + %td.old_line | ||
| 22 | + = link_to raw(type == "new" ? " " : line_old), "##{line_code}", id: line_code | ||
| 23 | + %td.new_line= link_to raw(type == "old" ? " " : line_new) , "##{line_code}", id: line_code | ||
| 24 | + %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line) | ||
| 25 | + - else | ||
| 26 | + %p.nothing_here_message No changes. |
app/views/projects/edit_tree/show.html.haml
| 1 | %h3.page-title Edit mode | 1 | %h3.page-title Edit mode |
| 2 | .file-editor | 2 | .file-editor |
| 3 | = form_tag(project_edit_tree_path(@project, @id), method: :put, class: "form-horizontal") do | 3 | = form_tag(project_edit_tree_path(@project, @id), method: :put, class: "form-horizontal") do |
| 4 | - .file-holder | 4 | + .file-holder.file |
| 5 | .file-title | 5 | .file-title |
| 6 | + .btn-group.js-edit-mode.left-options | ||
| 7 | + = link_to 'Edit', '#editor', class: 'active hover btn btn-tiny' | ||
| 8 | + = link_to editing_preview_title(@blob.name), '#preview', class: 'btn btn-tiny', 'data-preview-url' => preview_project_edit_tree_path(@project, @id) | ||
| 6 | %i.icon-file | 9 | %i.icon-file |
| 7 | %span.file_name | 10 | %span.file_name |
| 8 | = @path | 11 | = @path |
| @@ -13,7 +16,8 @@ | @@ -13,7 +16,8 @@ | ||
| 13 | .btn-group.tree-btn-group | 16 | .btn-group.tree-btn-group |
| 14 | = link_to "Cancel", @after_edit_path, class: "btn btn-tiny btn-cancel", data: { confirm: leave_edit_message } | 17 | = link_to "Cancel", @after_edit_path, class: "btn btn-tiny btn-cancel", data: { confirm: leave_edit_message } |
| 15 | .file-content.code | 18 | .file-content.code |
| 16 | - %pre#editor= @blob.data | 19 | + %pre.js-edit-mode-pane#editor= @blob.data |
| 20 | + .js-edit-mode-pane#preview.hide | ||
| 17 | 21 | ||
| 18 | .form-group.commit_message-group | 22 | .form-group.commit_message-group |
| 19 | = label_tag 'commit_message', class: "control-label" do | 23 | = label_tag 'commit_message', class: "control-label" do |
| @@ -45,3 +49,28 @@ | @@ -45,3 +49,28 @@ | ||
| 45 | $("#file-content").val(editor.getValue()); | 49 | $("#file-content").val(editor.getValue()); |
| 46 | $(".file-editor form").submit(); | 50 | $(".file-editor form").submit(); |
| 47 | }); | 51 | }); |
| 52 | + | ||
| 53 | + var editModePanes = $('.js-edit-mode-pane'), | ||
| 54 | + editModeLinks = $('.js-edit-mode a'); | ||
| 55 | + | ||
| 56 | + editModeLinks.click(function(event) { | ||
| 57 | + event.preventDefault(); | ||
| 58 | + | ||
| 59 | + var currentLink = $(this), | ||
| 60 | + paneId = currentLink.attr('href'), | ||
| 61 | + currentPane = editModePanes.filter(paneId); | ||
| 62 | + | ||
| 63 | + editModeLinks.removeClass('active hover'); | ||
| 64 | + currentLink.addClass('active hover'); | ||
| 65 | + editModePanes.hide(); | ||
| 66 | + | ||
| 67 | + if (paneId == '#preview') { | ||
| 68 | + $.post(currentLink.data('preview-url'), { content: editor.getValue() }, function(response) { | ||
| 69 | + currentPane.empty().append(response); | ||
| 70 | + currentPane.fadeIn(200); | ||
| 71 | + }) | ||
| 72 | + } else { | ||
| 73 | + currentPane.fadeIn(200); | ||
| 74 | + editor.focus() | ||
| 75 | + } | ||
| 76 | + }) |
config/routes.rb
| @@ -187,7 +187,9 @@ Gitlab::Application.routes.draw do | @@ -187,7 +187,9 @@ Gitlab::Application.routes.draw do | ||
| 187 | resources :blob, only: [:show, :destroy], constraints: {id: /.+/} | 187 | resources :blob, only: [:show, :destroy], constraints: {id: /.+/} |
| 188 | resources :raw, only: [:show], constraints: {id: /.+/} | 188 | resources :raw, only: [:show], constraints: {id: /.+/} |
| 189 | resources :tree, only: [:show], constraints: {id: /.+/, format: /(html|js)/ } | 189 | resources :tree, only: [:show], constraints: {id: /.+/, format: /(html|js)/ } |
| 190 | - resources :edit_tree, only: [:show, :update], constraints: {id: /.+/}, path: 'edit' | 190 | + resources :edit_tree, only: [:show, :update], constraints: { id: /.+/ }, path: 'edit' do |
| 191 | + post :preview, on: :member | ||
| 192 | + end | ||
| 191 | resources :new_tree, only: [:show, :update], constraints: {id: /.+/}, path: 'new' | 193 | resources :new_tree, only: [:show, :update], constraints: {id: /.+/}, path: 'new' |
| 192 | resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} | 194 | resources :commit, only: [:show], constraints: {id: /[[:alnum:]]{6,40}/} |
| 193 | resources :commits, only: [:show], constraints: {id: /(?:[^.]|\.(?!atom$))+/, format: /atom/} | 195 | resources :commits, only: [:show], constraints: {id: /(?:[^.]|\.(?!atom$))+/, format: /atom/} |
db/schema.rb
| @@ -323,7 +323,6 @@ ActiveRecord::Schema.define(version: 20140428105831) do | @@ -323,7 +323,6 @@ ActiveRecord::Schema.define(version: 20140428105831) do | ||
| 323 | t.integer "notification_level", default: 1, null: false | 323 | t.integer "notification_level", default: 1, null: false |
| 324 | t.datetime "password_expires_at" | 324 | t.datetime "password_expires_at" |
| 325 | t.integer "created_by_id" | 325 | t.integer "created_by_id" |
| 326 | - t.datetime "last_credential_check_at" | ||
| 327 | t.string "avatar" | 326 | t.string "avatar" |
| 328 | t.string "confirmation_token" | 327 | t.string "confirmation_token" |
| 329 | t.datetime "confirmed_at" | 328 | t.datetime "confirmed_at" |
| @@ -331,6 +330,7 @@ ActiveRecord::Schema.define(version: 20140428105831) do | @@ -331,6 +330,7 @@ ActiveRecord::Schema.define(version: 20140428105831) do | ||
| 331 | t.string "unconfirmed_email" | 330 | t.string "unconfirmed_email" |
| 332 | t.boolean "hide_no_ssh_key", default: false | 331 | t.boolean "hide_no_ssh_key", default: false |
| 333 | t.string "website_url", default: "", null: false | 332 | t.string "website_url", default: "", null: false |
| 333 | + t.datetime "last_credential_check_at" | ||
| 334 | end | 334 | end |
| 335 | 335 | ||
| 336 | add_index "users", ["admin"], name: "index_users_on_admin", using: :btree | 336 | add_index "users", ["admin"], name: "index_users_on_admin", using: :btree |
features/project/source/browse_files.feature
| @@ -29,3 +29,13 @@ Feature: Project Browse files | @@ -29,3 +29,13 @@ Feature: Project Browse files | ||
| 29 | Given I click on "Gemfile.lock" file in repo | 29 | Given I click on "Gemfile.lock" file in repo |
| 30 | And I click button "edit" | 30 | And I click button "edit" |
| 31 | Then I can edit code | 31 | Then I can edit code |
| 32 | + | ||
| 33 | + @javascript | ||
| 34 | + Scenario: I can see editing preview | ||
| 35 | + Given I click on "Gemfile.lock" file in repo | ||
| 36 | + And I click button "edit" | ||
| 37 | + And I edit code | ||
| 38 | + And I click link "Diff" | ||
| 39 | + Then I see diff | ||
| 40 | + | ||
| 41 | + |
features/steps/project/browse_files.rb
| @@ -41,6 +41,18 @@ class ProjectBrowseFiles < Spinach::FeatureSteps | @@ -41,6 +41,18 @@ class ProjectBrowseFiles < Spinach::FeatureSteps | ||
| 41 | page.evaluate_script('editor.getValue()').should == "GitlabFileEditor" | 41 | page.evaluate_script('editor.getValue()').should == "GitlabFileEditor" |
| 42 | end | 42 | end |
| 43 | 43 | ||
| 44 | + step 'I edit code' do | ||
| 45 | + page.execute_script('editor.setValue("GitlabFileEditor")') | ||
| 46 | + end | ||
| 47 | + | ||
| 48 | + step 'I click link "Diff"' do | ||
| 49 | + click_link 'Diff' | ||
| 50 | + end | ||
| 51 | + | ||
| 52 | + step 'I see diff' do | ||
| 53 | + page.should have_css '.line_holder.new' | ||
| 54 | + end | ||
| 55 | + | ||
| 44 | step 'I click on "new file" link in repo' do | 56 | step 'I click on "new file" link in repo' do |
| 45 | click_link 'new-file-link' | 57 | click_link 'new-file-link' |
| 46 | end | 58 | end |
lib/gitlab/diff_parser.rb
| @@ -4,9 +4,9 @@ module Gitlab | @@ -4,9 +4,9 @@ module Gitlab | ||
| 4 | 4 | ||
| 5 | attr_reader :lines, :new_path | 5 | attr_reader :lines, :new_path |
| 6 | 6 | ||
| 7 | - def initialize(diff) | ||
| 8 | - @lines = diff.diff.lines.to_a | ||
| 9 | - @new_path = diff.new_path | 7 | + def initialize(lines, new_path = '') |
| 8 | + @lines = lines | ||
| 9 | + @new_path = new_path | ||
| 10 | end | 10 | end |
| 11 | 11 | ||
| 12 | def each | 12 | def each |
| @@ -18,10 +18,7 @@ module Gitlab | @@ -18,10 +18,7 @@ module Gitlab | ||
| 18 | lines_arr.each do |line| | 18 | lines_arr.each do |line| |
| 19 | raw_line = line.dup | 19 | raw_line = line.dup |
| 20 | 20 | ||
| 21 | - next if line.match(/^\-\-\- \/dev\/null/) | ||
| 22 | - next if line.match(/^\+\+\+ \/dev\/null/) | ||
| 23 | - next if line.match(/^\-\-\- a/) | ||
| 24 | - next if line.match(/^\+\+\+ b/) | 21 | + next if filename?(line) |
| 25 | 22 | ||
| 26 | full_line = html_escape(line.gsub(/\n/, '')) | 23 | full_line = html_escape(line.gsub(/\n/, '')) |
| 27 | full_line = ::Gitlab::InlineDiff.replace_markers full_line | 24 | full_line = ::Gitlab::InlineDiff.replace_markers full_line |
| @@ -53,8 +50,17 @@ module Gitlab | @@ -53,8 +50,17 @@ module Gitlab | ||
| 53 | end | 50 | end |
| 54 | end | 51 | end |
| 55 | 52 | ||
| 53 | + def empty? | ||
| 54 | + @lines.empty? | ||
| 55 | + end | ||
| 56 | + | ||
| 56 | private | 57 | private |
| 57 | 58 | ||
| 59 | + def filename?(line) | ||
| 60 | + line.start_with?('--- /dev/null', '+++ /dev/null', '--- a', '+++ b', | ||
| 61 | + '--- /tmp/diffy', '+++ /tmp/diffy') | ||
| 62 | + end | ||
| 63 | + | ||
| 58 | def identification_type(line) | 64 | def identification_type(line) |
| 59 | if line[0] == "+" | 65 | if line[0] == "+" |
| 60 | "new" | 66 | "new" |