Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

unfold commits #7281

Merged
merged 1 commit into from Aug 15, 2014

Conversation

Projects
None yet
7 participants
Contributor

skv-headless commented Jul 9, 2014

unfold mov

TeatroIO commented Jul 9, 2014

I've prepared a stage. Click to open.

@houndci-bot houndci-bot commented on an outdated diff Jul 9, 2014

app/controllers/projects/blob_controller.rb
@@ -25,6 +25,20 @@ def destroy
end
end
+ def diff
+ @form = UnfoldForm.new(params)
+ @lines = @blob.data.lines[@form.since - 1..@form.to - 1]
+
+ if @form.bottom?
+ @match_line = ''
+ else
+ lines_length = @lines.length - 1
+ @match_line = "@@ -#{@form.since},#{lines_length} +#{@form.since},#{lines_length} @@"
@houndci-bot

houndci-bot Jul 9, 2014

Line is too long. [91/80]

@houndci-bot houndci-bot commented on the diff Jul 9, 2014

app/controllers/projects/blob_controller.rb
@@ -25,6 +25,20 @@ def destroy
end
end
+ def diff
+ @form = UnfoldForm.new(params)
+ @lines = @blob.data.lines[@form.since - 1..@form.to - 1]
+
+ if @form.bottom?
+ @match_line = ''
@houndci-bot

houndci-bot Jul 9, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot houndci-bot commented on an outdated diff Jul 9, 2014

app/forms/unfold_form.rb
@@ -0,0 +1,11 @@
+require_relative '../../lib/coercions/gt_one'
@houndci-bot

houndci-bot Jul 9, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot houndci-bot commented on an outdated diff Jul 9, 2014

app/forms/unfold_form.rb
@@ -0,0 +1,11 @@
+require_relative '../../lib/coercions/gt_one'
+
+class UnfoldForm
+ include Virtus.model
+
+ attribute :since, GtOne
+ attribute :to, GtOne
+ attribute :bottom, Boolean
+ attribute :unfold, Boolean, default: true
+ attribute :offset, Integer
+end
@houndci-bot

houndci-bot Jul 9, 2014

Final newline missing.

@houndci-bot houndci-bot commented on an outdated diff Jul 9, 2014

config/routes.rb
@@ -181,7 +181,9 @@
end
scope module: :projects do
- resources :blob, only: [:show, :destroy], constraints: {id: /.+/}
+ resources :blob, only: [:show, :destroy], constraints: {id: /.+/} do
@houndci-bot

houndci-bot Jul 9, 2014

Space inside { missing.
Space inside } missing.

@houndci-bot houndci-bot commented on the diff Jul 9, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step 'I unfold diff' do
@houndci-bot

houndci-bot Jul 9, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot houndci-bot commented on the diff Jul 9, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step 'I unfold diff' do
+ first('.js-unfold').click
@houndci-bot

houndci-bot Jul 9, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot houndci-bot commented on the diff Jul 9, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step 'I unfold diff' do
+ first('.js-unfold').click
+ end
+
+ step 'I should see additional file lines' do
@houndci-bot

houndci-bot Jul 9, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot houndci-bot commented on an outdated diff Jul 9, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step 'I unfold diff' do
+ first('.js-unfold').click
+ end
+
+ step 'I should see additional file lines' do
+ expect(first('.old_line')).to have_content('1')
@houndci-bot

houndci-bot Jul 9, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot houndci-bot commented on an outdated diff Jul 9, 2014

lib/coercions/gt_one.rb
@@ -0,0 +1,5 @@
+class GtOne < Virtus::Attribute
+ def coerce(value)
+ [1, value.to_i].max
+ end
+end
@houndci-bot

houndci-bot Jul 9, 2014

Final newline missing.

Owner

dosire commented Jul 9, 2014

Wow, awesome feature!

@dzaporozhets dzaporozhets commented on an outdated diff Jul 9, 2014

app/assets/javascripts/dispatcher.js.coffee
@@ -25,11 +25,18 @@ class Dispatcher
new Milestone()
when 'projects:issues:new', 'projects:merge_requests:new'
GitLab.GfmAutoComplete.setup()
+ when 'projects:merge_requests:index'
+ Diff()
@dzaporozhets

dzaporozhets Jul 9, 2014

Owner

why we need it for merge requests index page?

@dzaporozhets dzaporozhets commented on an outdated diff Jul 9, 2014

app/forms/unfold_form.rb
@@ -0,0 +1,11 @@
+require_relative '../../lib/coercions/gt_one'
+
+class UnfoldForm
@dzaporozhets

dzaporozhets Jul 9, 2014

Owner

this is the first and possible the only app/form class in the project. I dont like having a directory with single file in GitLab architecture. If you dont know where to put stuff - use libs directory. Reason for this - reduce amount of places to lookup for code.

Also remember that we dont even have virtus gem in Gemfile. So simple changing of dependencies can break this code. So please consider not use virtus or add such gem to Gemfile

Owner

dzaporozhets commented Jul 9, 2014

Awesome feature!

@houndci-bot houndci-bot commented on an outdated diff Jul 11, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step "I unfold diff" do
+ first(".js-unfold").click
+ end
+
+ step "I should see additional file lines" do
+ expect(first(".old_line")).to have_content('1')
@houndci-bot

houndci-bot Jul 11, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Contributor

MrKeiKun commented Jul 12, 2014

Awesome feature.

Question:
how many lines will it show on unfold?

Contributor

skv-headless commented Jul 12, 2014

Owner

dzaporozhets commented Jul 14, 2014

@skv-headless can you make it mergeable and tests green! I would like to merge this PR 😄

Contributor

skv-headless commented Jul 14, 2014

Sure. I need some time for refactoring.

@houndci-bot houndci-bot commented on an outdated diff Jul 14, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step "I unfold diff" do
+ first(".js-unfold").click
+ end
+
+ step 'I should see additional file lines' do
@houndci-bot

houndci-bot Jul 14, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot houndci-bot commented on an outdated diff Jul 14, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step "I unfold diff" do
+ first(".js-unfold").click
+ end
+
+ step 'I should see additional file lines' do
+ expect(first('.text-file')).to have_content('.bundle')
@houndci-bot

houndci-bot Jul 14, 2014

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Owner

dosire commented Jul 15, 2014

@skv-headless Thanks for contributing this awesome feature! By the way, the merge window for 7.1 is closing today.

Contributor

skv-headless commented Jul 15, 2014

It's ok.

Owner

dblessing commented Jul 24, 2014

@skv-headless very cool! Ping one of us when it's rebased.

@dzaporozhets dzaporozhets commented on an outdated diff Jul 25, 2014

...views/projects/commits/diffs/_view_file_btn.html.haml
@@ -0,0 +1,4 @@
+= link_to project_blob_path(project, tree_join(commit.id, diff.new_path)),
+ { class: 'btn btn-small view-file js-view-file' } do
+ View file @
+ %span.commit-short-id= commit.short_id(6)
@dzaporozhets

dzaporozhets Jul 25, 2014

Owner

@skv-headless please move this logic into helper. It will look better as helper method than haml template

Owner

dzaporozhets commented Jul 25, 2014

@skv-headless I made one note according to code. Can you please fix it and rebase on master? It will be cool to have this feature in master

Owner

dzaporozhets commented Jul 28, 2014

ping @skv-headless :)

@houndci-bot houndci-bot commented on an outdated diff Aug 1, 2014

app/helpers/commits_helper.rb
@@ -231,4 +231,16 @@ def commit_person_link(commit, options = {})
def diff_file_mode_changed?(diff)
diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
end
+
+ def unfold_bottom_class(bottom)
+ (bottom) ? "js-unfold-bottom" : ""
+ end
+
+ def view_file_btn(commit_sha, diff, project)
+ link_to project_blob_path(project, tree_join(commit_sha, diff.new_path)),
+ { class: 'btn btn-small view-file js-view-file' } do
@houndci-bot

houndci-bot Aug 1, 2014

Redundant curly braces around a hash parameter.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

app/controllers/projects/blob_controller.rb
@@ -25,6 +25,21 @@ def destroy
end
end
+ def diff
+ @form = UnfoldForm.new(params)
+ @lines = @blob.data.lines[@form.since - 1..@form.to - 1]
+
+ if @form.bottom?
+ @match_line = ""
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

app/controllers/projects/blob_controller.rb
@@ -25,6 +25,21 @@ def destroy
end
end
+ def diff
+ @form = UnfoldForm.new(params)
+ @lines = @blob.data.lines[@form.since - 1..@form.to - 1]
+
+ if @form.bottom?
+ @match_line = ""
+ else
+ lines_length = @lines.length - 1
+ line = [@form.since, lines_length].join(",")
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

app/helpers/commits_helper.rb
@@ -232,4 +232,16 @@ def commit_person_link(commit, options = {})
def diff_file_mode_changed?(diff)
diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
end
+
+ def unfold_bottom_class(bottom)
+ (bottom) ? "js-unfold-bottom" : ""
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

app/helpers/commits_helper.rb
@@ -232,4 +232,16 @@ def commit_person_link(commit, options = {})
def diff_file_mode_changed?(diff)
diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
end
+
+ def unfold_bottom_class(bottom)
+ (bottom) ? "js-unfold-bottom" : ""
+ end
+
+ def view_file_btn(commit_sha, diff, project)
+ link_to project_blob_path(project, tree_join(commit_sha, diff.new_path)),
+ { class: 'btn btn-small view-file js-view-file' } do
+ raw("View file @") + content_tag(:span, commit_sha[0..6],
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

app/helpers/commits_helper.rb
@@ -232,4 +232,16 @@ def commit_person_link(commit, options = {})
def diff_file_mode_changed?(diff)
diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode
end
+
+ def unfold_bottom_class(bottom)
+ (bottom) ? "js-unfold-bottom" : ""
+ end
+
+ def view_file_btn(commit_sha, diff, project)
+ link_to project_blob_path(project, tree_join(commit_sha, diff.new_path)),
+ { class: 'btn btn-small view-file js-view-file' } do
+ raw("View file @") + content_tag(:span, commit_sha[0..6],
+ class: "commit-short-id")
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step "I unfold diff" do
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step "I unfold diff" do
+ first(".js-unfold").click
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step "I unfold diff" do
+ first(".js-unfold").click
+ end
+
+ step "I should see additional file lines" do
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

features/steps/project/merge_requests.rb
@@ -239,6 +239,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps
end
end
+ step "I unfold diff" do
+ first(".js-unfold").click
+ end
+
+ step "I should see additional file lines" do
+ expect(first(".text-file")).to have_content(".bundle")
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@houndci-bot houndci-bot commented on an outdated diff Aug 2, 2014

lib/unfold_form.rb
@@ -0,0 +1,11 @@
+require_relative "gt_one_coercion"
@houndci-bot

houndci-bot Aug 2, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

Contributor

skv-headless commented Aug 2, 2014

@randx rebased. houndci confusing me about quotes 😊

@dzaporozhets dzaporozhets commented on an outdated diff Aug 10, 2014

app/views/projects/commits/_diff_file.html.haml
@@ -26,10 +26,7 @@
Edit
&nbsp;
- = link_to project_blob_path(project, tree_join(@commit.id, diff.new_path)), { class: 'btn btn-small view-file' } do
- View file @
- %span.commit-short-id= @commit.short_id(6)
-
+ = view_file_btn(@commit.parent_id, diff, project)
@dzaporozhets

dzaporozhets Aug 10, 2014

Owner

@skv-headless it was commit.id before but you changed to commit.parent_id. Can you explain?

Contributor

skv-headless commented Aug 14, 2014

@randx thanks for note about mistake, fixed and rebased

@dzaporozhets dzaporozhets added a commit that referenced this pull request Aug 15, 2014

@dzaporozhets dzaporozhets Merge pull request #7281 from skv-headless/diff-unfold
unfold commits
7120af7

@dzaporozhets dzaporozhets merged commit 7120af7 into gitlabhq:master Aug 15, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build could not complete due to an error
Details
Owner

dzaporozhets commented Aug 15, 2014

@skv-headless thank you!

Owner

dosire commented Aug 17, 2014

@skv-headless ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment