Mailinglist Archive: obs-commits (345 mails)

< Previous Next >
[obs-commits] [PATCH] [webui] Rdiff overhaul.
From: Sascha Peilicke <saschpe@xxxxxxx>

Uses newer source diff partial (i.e. CodeWarrior2). The parameter 'rev'
is now used instead of 'commit', as it always means the rev of the diff
source. If there are 'oproject' and 'opackage' parameters, it is clear
that a diff to a link target is to be shown.
---
src/webui/app/controllers/package_controller.rb | 68 +++++++++++++-------
src/webui/app/views/package/_commit_item.html.erb | 4 +-
src/webui/app/views/package/rdiff.rhtml | 60 +++++++-----------
src/webui/app/views/package/view_file.rhtml | 4 +-
4 files changed, 72 insertions(+), 64 deletions(-)

diff --git a/src/webui/app/controllers/package_controller.rb
b/src/webui/app/controllers/package_controller.rb
index f2b9e1f..9db1ae3 100644
--- a/src/webui/app/controllers/package_controller.rb
+++ b/src/webui/app/controllers/package_controller.rb
@@ -290,46 +290,66 @@ class PackageController < ApplicationController

def rdiff
required_parameters :project, :package
- if params[:commit]
- @rev = params[:commit]
+ @last_rev = Package.current_rev(@project, @package.name)
+ if params[:oproject] and params[:opackage]
+ @oproject, @opackage = params[:oproject], params[:opackage]
+ @last_req = BsRequest.find_last_request(:targetproject => @oproject,
:targetpackage => @opackage, :sourceproject => params[:project], :sourcepackage
=> params[:package])
+ if @last_req and @last_req.state.name != "declined"
+ @last_req = nil # ignore all !declined
+ end
+ end
+ if params[:rev]
+ @rev = params[:rev]
else
- @rev = Package.current_rev(@project, @package.name)
- required_parameters :opackage, :oproject
- @opackage = params[:opackage]
- @oproject = params[:oproject]
+ @rev = @last_rev
end
- @rdiff = ''
- path =
"/source/#{CGI.escape(params[:project])}/#{CGI.escape(params[:package])}?cmd=diff&unified=1"
+
+ path =
"/source/#{CGI.escape(params[:project])}/#{CGI.escape(params[:package])}?cmd=diff&view=xml"
path += "&linkrev=#{CGI.escape(params[:linkrev])}" if params[:linkrev]
path += "&rev=#{CGI.escape(@rev)}" if @rev
path += "&oproject=#{CGI.escape(@oproject)}" if @oproject
path += "&opackage=#{CGI.escape(@opackage)}" if @opackage
- path += "&orev=#{CGI.escape(@orev)}" if @orev
+ path += "&orev=#{CGI.escape(params[:orev])}" if params[:orev]
begin
- @rdiff = frontend.transport.direct_http URI(path + "&expand=1"), :method
=> "POST", :data => ""
+ rdiff = frontend.transport.direct_http URI(path + "&expand=1"), :method
=> "POST", :data => ""
rescue ActiveXML::Transport::NotFoundError => e
- message, code, api_exception =
ActiveXML::Transport.extract_error_message e
- flash.now[:error] = message
+ flash.now[:error], _, _ = ActiveXML::Transport.extract_error_message(e)
return
rescue ActiveXML::Transport::Error => e
- message, code, api_exception =
ActiveXML::Transport.extract_error_message e
- flash.now[:warn] = message
+ flash.now[:warn], _, _ = ActiveXML::Transport.extract_error_message(e)
begin
- @rdiff = frontend.transport.direct_http URI(path + "&expand=0"),
:method => "POST", :data => ""
+ rdiff = frontend.transport.direct_http URI(path + "&expand=0"),
:method => "POST", :data => ""
rescue ActiveXML::Transport::Error => e
- message, code, api_exception =
ActiveXML::Transport.extract_error_message e
+ message, _, _ = ActiveXML::Transport.extract_error_message e
flash.now[:warn] = nil
flash.now[:error] = "Error getting diff: " + message
return
end
end
- if not params[:commit]
- @lastreq = BsRequest.find_last_request(:targetproject => @oproject,
:targetpackage => @opackage,
- :sourceproject => params[:project], :sourcepackage => params[:package])
- if @lastreq and @lastreq.state.name != "declined"
- @lastreq = nil # ignore all !declined
+
+ result = ActiveXML::Base.new(rdiff)
+ # Sort files into categories by their ending and add all of them to a
hash. We
+ # will later use the sorted and concatenated categories as key index into
the per action file hash.
+ changes_file_keys, spec_file_keys, patch_file_keys, other_file_keys = [],
[], [], []
+ @files = {}
+ result.each('files/file') do |file_element|
+ if file_element.new
+ filename = file_element.new.name.to_s
+ elsif file_element.old # in case of deleted files
+ filename = file_element.old.name.to_s
end
+ if filename.ends_with?('.spec')
+ spec_file_keys << filename
+ elsif filename.ends_with?('.changes')
+ changes_file_keys << filename
+ elsif filename.match(/.*.(patch|diff|dif)/)
+ patch_file_keys << filename
+ else
+ other_file_keys << filename
+ end
+ @files[filename] = file_element
end
+ @filenames = changes_file_keys.sort + spec_file_keys.sort +
patch_file_keys.sort + other_file_keys.sort;
end

def wizard_new
@@ -773,11 +793,11 @@ class PackageController < ApplicationController

def view_file
@filename = params[:file] || ''
- @srcmd5 = params[:srcmd5]
+ @rev = params[:rev]
@addeditlink = false
if @package.can_edit?( session[:login] )
begin
- files = @package.files(@srcmd5, params[:expand])
+ files = @package.files(@rev, params[:expand])
rescue ActiveXML::Transport::Error => e
files = []
end
@@ -789,7 +809,7 @@ class PackageController < ApplicationController
end
end
begin
- @file = frontend.get_source(:project => @project.to_s, :package =>
@package.to_s, :filename => @filename, :rev => @srcmd5)
+ @file = frontend.get_source(:project => @project.to_s, :package =>
@package.to_s, :filename => @filename, :rev => @rev)
rescue ActiveXML::Transport::NotFoundError => e
flash[:error] = "File not found: #{@filename}"
redirect_to :action => :files, :package => @package, :project =>
@project and return
diff --git a/src/webui/app/views/package/_commit_item.html.erb
b/src/webui/app/views/package/_commit_item.html.erb
index 8a43e67..469c255 100644
--- a/src/webui/app/views/package/_commit_item.html.erb
+++ b/src/webui/app/views/package/_commit_item.html.erb
@@ -23,9 +23,9 @@
<% end %>
<%= link_to(image_tag('icons/brick.png'), { :controller => :package, :action
=> :rdiff,
:package => @package, :project => @project,
- :commit => commit[:revision], :linkrev => 'base'}, :style => "margin-left:
2em") %>
+ :rev => commit[:revision], :linkrev => 'base'}, :style => "margin-left:
2em") %>
<%= link_to('Show diff', :controller => :package, :action => :rdiff,
- :package => @package, :project => @project, :commit => commit[:revision],
:linkrev => 'base') %>
+ :package => @package, :project => @project, :rev => commit[:revision],
:linkrev => 'base') %>
<% else %>
<i>Revision <%=rev.to_s%> not found</i>
<% end %>
diff --git a/src/webui/app/views/package/rdiff.rhtml
b/src/webui/app/views/package/rdiff.rhtml
index 666bba0..e87562a 100644
--- a/src/webui/app/views/package/rdiff.rhtml
+++ b/src/webui/app/views/package/rdiff.rhtml
@@ -1,48 +1,36 @@
-<% @pagetitle = "Changes of Commit #{params[:commit]}"
- package_bread_crumb("Changes")
--%>
+<% @pagetitle = "Changes" %>
+<% package_bread_crumb("Changes") %>

<%= render :partial => "tabs" %>

-<% if params[:commit] %>
- <h3><%= h @pagetitle %></h3>
+<% if @oproject and @opackage %>
+ <h3>Difference Between Revision <%= @rev %> and <%= link_to(h(@oproject +
"/" + @opackage), :action => :show, :package => @opackage, :project =>
@oproject) %></h3>
<% else %>
-<!-- this is causing a test suite failure when pointing to remote projects.
- <h2>Diff between <%= link_to(h(@oproject + "/" + @opackage), :action =>
:show, :package => @opackage, :project => @oproject) %> and <%=h @project.name
+ "/" + @package.name %></h2>
--->
+ <h3>Changes of Revision <%= @rev %></h3>
<% end %>

-<% if @rdiff.empty? %>
- <p><i>No difference</i></p>
-<% else %>
- <% if @addeditlink %>
- <p>
- <% link_to(image_tag("icon/package_edit.png", :title => "Edit diff"),
:action => :edit_file, :project => @project, :package => @package, :file =>
@filename) %>
- <% link_to("Edit diff", :action => :edit_file, :project => @project,
:package => @package, :file => @filename) %>
- </p>
- <% end %>
-
- <%= render :partial => "shared/editor", :locals => {:text => @rdiff, :mode
=> 'diff'} %>
+<% if @last_req %>
+<p>The previous request <%= @last_req %> was declined <%= "%s by %s:" %
[fuzzy_time_string(@last_req.state.when), @last_req.state.who ] %>: <%=
@last_req.state.comment %></p>
+<% end %>

- <% if @lastreq %>
- <h3>The last request (<%= @lastreq.value(:id) %>) was declined <%= "%s by
%s:" % [fuzzy_time_string(@lastreq.state.when), @lastreq.state.who ] %></h3>
- <p><%= @lastreq.state.comment %></p>
+<% if session[:login] %>
+ <% if @oproject and @opackage %>
+ <% msg = "Submit to #{h(@oproject.to_s + '/' + @opackage.to_s)}" %>
+ <% target_project, target_package = @oproject, @opackage %>
+ <% elsif @rev != @last_rev %>
+ <% msg = "Revert #{h(@project.to_s + '/' + @package.to_s)} to revision
#{@rev}" %>
+ <% target_project, target_package = @project, nil %>
+ <% else %>
+ <% msg = nil %>
<% end %>
-
- <% if session[:login] %>
- <h3>Actions</h3>
- <% if params[:commit] %>
- <% msg = "Revert #{h(@project.to_s + ' / ' + @package.to_s)} to this
revision" %>
- <% target_project, target_package = @project, nil %>
- <% else %>
- <% msg = "Submit these changes to #{h(@oproject.to_s + ' / ' +
@opackage.to_s)}" %>
- <% target_project, target_package = @oproject, @opackage %>
- <% end %>
+ <% if msg %>
<p>
- <%= link_to_remote(image_tag('icons/package_go.png', :title => msg),
- :url => {:action => 'submit_request_dialog', :project => @project,
:package => @package, :targetproject => target_project, :targetpackage =>
target_package, :revision => @rev, :readonly => true}) %>
- <%= link_to_remote(msg,
- :url => {:action => 'submit_request_dialog', :project => @project,
:package => @package, :targetproject => target_project, :targetpackage =>
target_package, :revision => @rev, :readonly => true}) %>
+ <%= link_to_remote(image_tag('icons/package_go.png', :title => msg),
+ :url => {:action => 'submit_request_dialog', :project => @project,
:package => @package, :targetproject => target_project, :targetpackage =>
target_package, :revision => @rev, :readonly => true}) %>
+ <%= link_to_remote(msg,
+ :url => {:action => 'submit_request_dialog', :project => @project,
:package => @package, :targetproject => target_project, :targetpackage =>
target_package, :revision => @rev, :readonly => true}) %>
</p>
<% end %>
<% end %>
+
+<%= render(:partial => 'shared/sourcediff', :locals => {:filenames =>
@filenames, :files => @files, :source => {:project => @project, :package =>
@package, :rev => @rev}}) %>
diff --git a/src/webui/app/views/package/view_file.rhtml
b/src/webui/app/views/package/view_file.rhtml
index 8ef65d6..ec6a414 100644
--- a/src/webui/app/views/package/view_file.rhtml
+++ b/src/webui/app/views/package/view_file.rhtml
@@ -1,12 +1,12 @@
<% @pagetitle = "File #{@filename} of Package #{@package}" %>
-<% package_bread_crumb(link_to('Files', :action => :files, :project =>
@project, :package => @package, :rev => @srcmd5), h(@filename)) %>
+<% package_bread_crumb(link_to('Files', :action => :files, :project =>
@project, :package => @package, :rev => @rev), h(@filename)) %>

<%= render :partial => "tabs" %>

<h3><%= @pagetitle %></h3>
<div style="margin-left: 15px; margin-right: 15px;">
<% if @addeditlink %>
- <%= render :partial => "shared/editor", :locals => {:text => @file, :mode
=> guess_code_class(@filename), :save => {:url => {:controller => 'package',
:action => 'save_modified_file'}, :method => 'POST', :data => {:project =>
@project, :package => @package, :file => '@@@', :comment => '', :filename =>
@filename, :srcmd5 => @srcmd5}}} %>
+ <%= render :partial => "shared/editor", :locals => {:text => @file, :mode
=> guess_code_class(@filename), :save => {:url => {:controller => 'package',
:action => 'save_modified_file'}, :method => 'POST', :data => {:project =>
@project, :package => @package, :file => '@@@', :comment => '', :filename =>
@filename, :rev => @rev}}} %>
<%# TODO: Provide a comments field thru a callback!! %>
<% else %>
<%= render :partial => "shared/editor", :locals => {:text => @file, :mode
=> guess_code_class(@filename), :read_only => true} %>
--
1.7.7

--
To unsubscribe, e-mail: obs-commits+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: obs-commits+owner@xxxxxxxxxxxx

< Previous Next >