From 9ca39bf534965ff6257c00b2c4d18e8094ade9d5 Mon Sep 17 00:00:00 2001 From: Ryan Lerch Date: May 03 2016 08:45:59 +0000 Subject: [PATCH 1/3] started playing wiht teh changes summary --- diff --git a/pagure/static/pagure.css b/pagure/static/pagure.css index b0cc905..9339687 100644 --- a/pagure/static/pagure.css +++ b/pagure/static/pagure.css @@ -204,6 +204,11 @@ right:6px!important; text-overflow: ellipsis; } +.ellipsis-left +{ + direction:rtl; +} + .projecticon .oi { font-size: 3.5rem; @@ -509,3 +514,8 @@ color: #999; th[data-sort] { cursor: pointer; } + +.pr-changes-description +{ + font-size: 0.8em; +} diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index b9e42fd..219ee99 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -238,8 +238,8 @@ -
{% if authenticated and pull_request %}
Changes summary
+ + + {% macro changeschangedfile(filepath, added, removed) -%} +
+
+{{added}} -{{removed}}
+
file changed
+
+
+ {{filepath}} +
+ {%- endmacro %} + {% macro changesrenamedfile(oldfilepath, newfilepath, added, removed) -%} +
+
+{{added}} -{{removed}}
+
file renamed
+
+
+ {{oldfilepath}}
{{newfilepath}} +
+ {%- endmacro %} + {% macro changesdeletedfile(filepath, added, removed) -%} +
+
-{{removed}}
+
file deleted
+
+
+ {{filepath}} +
+ {%- endmacro %} + {% macro changesaddedfile(filepath, added, removed) -%} +
+
+{{added}}
+
file added
+
+
+ {{filepath}} +
+ {%- endmacro %}
{% for patch in diff %}
- - {%- if patch | hasattr('new_file_path') -%} - {%- if patch.new_file_path == patch.old_file_path -%} - {{ patch.new_file_path }} - {%- else -%} - {{ patch.new_file_path }} (Renamed from "{{ patch.old_file_path }}") - {%- endif -%} - {%- elif patch | hasattr('delta') -%} - {%- if patch.delta.new_file.path == patch.delta.old_file.path -%} - {{ patch.delta.new_file.path }} - {%- else -%} - {{ patch.delta.new_file.path }} (Renamed from "{{ patch.delta.old_file.path }}") - {%- endif -%} + + {% if patch | hasattr('additions') %} + {# Version of pygit2 -0.21.4 -- F21/EL7 #} + {% set linesadded = patch.additions %} + {% set linesremoved = patch.deletions %} + {% else %} + {# Version of pygit2 -0.23.0 -- F23 #} + {% set linesadded = patch.line_stats[1] %} + {% set linesremoved = patch.line_stats[2] %} + {% endif %} + + {% if patch | hasattr('new_file_path') %} + {%- if patch.new_file_path == patch.old_file_path -%} + {{ changeschangedfile(patch.new_file_path, linesadded, linesremoved) }} + {%- else -%} + {{changesrenamedfile(patch.old_file_path, patch.new_file_path, linesadded, linesremoved)}} {%- endif -%} - -
- {% if patch | hasattr('additions') %} - {# Version of pygit2 -0.21.4 -- F21/EL7 #} - {% if (patch.additions + patch.deletions) %} - - {% if patch.additions > 0 %}+{{ patch.additions }}{% endif %} - {% if patch.deletions > 0 %}-{{ patch.deletions }}{% endif %} - - {% endif %} - {% else %} - {# Version of pygit2 -0.23.0 -- F23 #} - {% if (patch.line_stats[1] + patch.line_stats[2]) %} - - {% if patch.line_stats[1] > 0 %}+{{ patch.line_stats[1] }}{% endif %} - {% if patch.line_stats[2] > 0 %}-{{ patch.line_stats[2] }}{% endif %} - - {% endif %} - {% endif %} -
+ {%- elif patch | hasattr('delta') -%} + {%- if patch.delta.new_file.path == patch.delta.old_file.path -%} + {%- if patch.delta.new_file.mode == 0 and patch.delta.old_file.mode == 33188 -%} + {{ changesdeletedfile(patch.delta.new_file.path, linesadded, linesremoved) }} + {%-elif patch.delta.new_file.mode == 33188 and patch.delta.old_file.mode == 0 -%} + {{ changesaddedfile(patch.delta.new_file.path, linesadded, linesremoved) }} + {%-elif patch.delta.new_file.mode == 33188 and patch.delta.old_file.mode == 33188 -%} + {{ changeschangedfile(patch.delta.new_file.path, linesadded, linesremoved) }} + {%-endif-%} + {%- else -%} + {{changesrenamedfile(patch.delta.old_file.path, patch.delta.new_file.path, linesadded, linesremoved)}} + {%- endif -%} + {%- endif -%}
{% endfor %}
From ac4e97b44bdfefd7c51216bbe1f70eca0b3a83a8 Mon Sep 17 00:00:00 2001 From: Ryan Lerch Date: May 03 2016 08:45:59 +0000 Subject: [PATCH 2/3] finalized changes to the PR changes summary and diff --- diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html index 219ee99..f7d4047 100644 --- a/pagure/templates/pull_request.html +++ b/pagure/templates/pull_request.html @@ -248,7 +248,9 @@ {% endif %} {% if diff %} + {% set count = 0 %} {% for patch in diff %} + {% set count = count+1 %} {% if patch |hasattr('new_id') %} {% set patch_new_id = patch.new_id %} {% elif patch |hasattr('delta') %} @@ -256,7 +258,6 @@ {% else %} {% set patch_new_id = patch.new_oid %} {% endif %} - {% if patch | hasattr('new_file_path') %} {% set patch_new_file_path = patch.new_file_path -%} {% if patch.new_file_path != patch.old_file_path %} @@ -269,38 +270,101 @@ {%- endif -%} {%- endif -%} -
-
+ +
+
- {%- if patch_new_file_path == patch_old_file_path -%} - {{ patch_new_file_path }} (Renamed from "{{ patch_old_file_path }}") - {%- else -%} - {{ patch_new_file_path }} - {%- endif -%} + {% if patch | hasattr('additions') %} + {# Version of pygit2 -0.21.4 -- F21/EL7 #} + {% set linesadded = patch.additions %} + {% set linesremoved = patch.deletions %} + {% else %} + {# Version of pygit2 -0.23.0 -- F23 #} + {% set linesadded = patch.line_stats[1] %} + {% set linesremoved = patch.line_stats[2] %} + {% endif %} + + {% macro lineschanged(added, removed) -%} + {%if added%} + +{{linesadded}} + {%endif%} + {%if removed%} + -{{linesremoved}} + {%endif%} + {%endmacro%} + {% macro viewfilelink(filepath)%} {% if pull_request %} - View + title="View file as of {{ patch_new_id|short }}">{{filepath}} + {% endmacro %} + + {% if patch | hasattr('new_file_path') %} + {%- if patch.new_file_path == patch.old_file_path -%} + {{ viewfilelink(patch.new_file_path)}} {{lineschanged(True, True)}} + {%- else -%} + {{lineschanged(True, True)}} +
+ {{patch.old_file_path}}
+ {{viewfilelink(patch.new_file_path)}} +
+
file removed
+ {%- endif -%} + {%- elif patch | hasattr('delta') -%} + {%- if patch.delta.new_file.path == patch.delta.old_file.path -%} + {%- if patch.delta.new_file.mode == 0 and patch.delta.old_file.mode == 33188 -%} + {% set patchtype = "removed"%} +
{{viewfilelink(patch.delta.new_file.path)}} {{lineschanged(False, True)}}
+
file removed
+ {%-elif patch.delta.new_file.mode == 33188 and patch.delta.old_file.mode == 0 -%} + {% set patchtype = "added"%} +
{{viewfilelink(patch.delta.new_file.path)}} {{lineschanged(True, False)}}
+
file added
+ {%-elif patch.delta.new_file.mode == 33188 and patch.delta.old_file.mode == 33188 -%} + {% set patchtype = "changed"%} +
+ {{ viewfilelink(patch.delta.new_file.path)}} {{lineschanged(True, True)}} +
+
file changed
+ {%-endif-%} + {%- else -%} + {% set patchtype = "moved"%} + {{lineschanged(True, True)}} +
+ {{patch.delta.old_file.path}}
+ {{viewfilelink(patch.delta.new_file.path)}} +
+
file moved
+ {%- endif -%} + {%- endif -%}
- + {% if patchtype == "moved" and linesadded == 0 and linesremoved == 0%} +
+
file was moved with no change to the file
+
+ {% elif patchtype == "added" and linesadded == 0 %} +
+
empty file added
+
+ {% else %} {% autoescape false %} {{ patch | patch_to_diff | html_diff | format_loc( filename=patch_new_file_path, @@ -309,6 +373,7 @@ index=loop.index, tree_id=diff_commits[0].tree.id)}} {% endautoescape %} + {% endif %}
@@ -505,46 +570,47 @@
Changes summary
- - - {% macro changeschangedfile(filepath, added, removed) -%} + {% macro changeschangedfile(filepath, added, removed, diffanchor) -%}
+{{added}} -{{removed}}
file changed
- {{filepath}} + {{filepath}}
{%- endmacro %} - {% macro changesrenamedfile(oldfilepath, newfilepath, added, removed) -%} + {% macro changesrenamedfile(oldfilepath, newfilepath, added, removed, diffanchor) -%}
+{{added}} -{{removed}}
file renamed
- {{oldfilepath}}
{{newfilepath}} + {{oldfilepath}}
+ {{newfilepath}}
{%- endmacro %} - {% macro changesdeletedfile(filepath, added, removed) -%} + {% macro changesdeletedfile(filepath, added, removed, diffanchor) -%}
-{{removed}}
-
file deleted
+
file removed
- {{filepath}} + {{filepath}}
{%- endmacro %} - {% macro changesaddedfile(filepath, added, removed) -%} + {% macro changesaddedfile(filepath, added, removed, diffanchor) -%}
+{{added}}
file added
- {{filepath}} + {{filepath}}
{%- endmacro %}
+ {% set count = 0 %} {% for patch in diff %} + {% set count = count+1 %}
{% if patch | hasattr('additions') %} @@ -559,21 +625,21 @@ {% if patch | hasattr('new_file_path') %} {%- if patch.new_file_path == patch.old_file_path -%} - {{ changeschangedfile(patch.new_file_path, linesadded, linesremoved) }} + {{ changeschangedfile(patch.new_file_path, linesadded, linesremoved, count) }} {%- else -%} - {{changesrenamedfile(patch.old_file_path, patch.new_file_path, linesadded, linesremoved)}} + {{changesrenamedfile(patch.old_file_path, patch.new_file_path, linesadded, linesremoved, count)}} {%- endif -%} {%- elif patch | hasattr('delta') -%} {%- if patch.delta.new_file.path == patch.delta.old_file.path -%} {%- if patch.delta.new_file.mode == 0 and patch.delta.old_file.mode == 33188 -%} - {{ changesdeletedfile(patch.delta.new_file.path, linesadded, linesremoved) }} + {{ changesdeletedfile(patch.delta.new_file.path, linesadded, linesremoved, count) }} {%-elif patch.delta.new_file.mode == 33188 and patch.delta.old_file.mode == 0 -%} - {{ changesaddedfile(patch.delta.new_file.path, linesadded, linesremoved) }} + {{ changesaddedfile(patch.delta.new_file.path, linesadded, linesremoved, count) }} {%-elif patch.delta.new_file.mode == 33188 and patch.delta.old_file.mode == 33188 -%} - {{ changeschangedfile(patch.delta.new_file.path, linesadded, linesremoved) }} + {{ changeschangedfile(patch.delta.new_file.path, linesadded, linesremoved, count) }} {%-endif-%} {%- else -%} - {{changesrenamedfile(patch.delta.old_file.path, patch.delta.new_file.path, linesadded, linesremoved)}} + {{changesrenamedfile(patch.delta.old_file.path, patch.delta.new_file.path, linesadded, linesremoved, count)}} {%- endif -%} {%- endif -%}
From 15a1bca167cfba0b76e58af92b02bd3ec8aaf666 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: May 03 2016 12:07:10 +0000 Subject: [PATCH 3/3] Fix unit-tests --- diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 96ffc6f..6b8c2fd 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -252,7 +252,7 @@ class PagureFlaskForktests(tests.Modeltests): '

PR#1\n' ' PR from the feature branch\n

', output.data) self.assertIn( - 'title="View file as of 2a552b">View', output.data) + 'title="View file as of 2a552b">sources', output.data) @patch('pagure.lib.notify.send_email') def test_merge_request_pull_FF(self, send_email): @@ -280,7 +280,7 @@ class PagureFlaskForktests(tests.Modeltests): '

PR#1\n' ' PR from the feature branch\n

', output.data) self.assertIn( - 'title="View file as of 2a552b">View', output.data) + 'title="View file as of 2a552b">sources', output.data) # Wrong project data = { @@ -545,7 +545,7 @@ class PagureFlaskForktests(tests.Modeltests): ' ', output.data) self.assertIn('
Merged by\n', output.data) self.assertIn( - 'title="View file as of 2a552b">View', output.data) + 'title="View file as of 2a552b">sources', output.data) @patch('pagure.lib.notify.send_email') def test_request_pull_disabled(self, send_email): @@ -1768,7 +1768,7 @@ index 0000000..2a552bb '

PR#1\n' ' PR from the feature branch\n

', output.data) self.assertIn( - 'title="View file as of 2a552b">View', output.data) + 'title="View file as of 2a552b">sources', output.data) # Wrong project data = { diff --git a/tests/test_pagure_flask_ui_slash_branch_name.py b/tests/test_pagure_flask_ui_slash_branch_name.py index 61cb737..10e50cb 100644 --- a/tests/test_pagure_flask_ui_slash_branch_name.py +++ b/tests/test_pagure_flask_ui_slash_branch_name.py @@ -361,7 +361,11 @@ class PagureFlaskSlashInBranchtests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertEqual( output.data.count('.gitignore', output.data) + self.assertIn( + '' + '+1', output.data) + self.assertIn( + '
file added
', output.data) user = tests.FakeUser() with tests.user_set(pagure.APP, user): @@ -369,7 +373,11 @@ class PagureFlaskSlashInBranchtests(tests.Modeltests): self.assertEqual(output.status_code, 200) self.assertEqual( output.data.count('.gitignore', output.data) + self.assertIn( + '' + '+1', output.data) + self.assertIn( + '
file added
', output.data) if __name__ == '__main__':