pagure

Clone

#4951 Fix line numbering on pull requests
Merged 4 years ago by pingou. Opened 4 years ago by jlanda.
jlanda/pagure fix-pr-diff-line-number  into  master

file modified
-1 
@@ -164,7 +164,6 @@ 
 

                        commit=patchstats["new_id"],
 

                        prequest=pull_request,
 

                        index=loop.index,
 

-                       isprdiff=True,
 

                        tree_id=diff_commits[0].tree.id)}}
 

            </div>
 

            {% endautoescape %}

file modified
+42 -59 
@@ -149,38 +149,33 @@ 
 

      for key in comments:
 

          comments[key] = sorted(comments[key], key=lambda obj: obj.date_created)
 

  

-     if not index:
 

-         index = ""
 

+     if isinstance(filename, str) and six.PY2:
 

+         filename = filename.decode("UTF-8")
 

  

      cnt = 1
 

      for line in loc.split("\n"):
 

-         if filename and commit:
 

-             if isinstance(filename, str) and six.PY2:
 

-                 filename = filename.decode("UTF-8")
 

- 

-             if isprdiff and (
 

-                 line.startswith("@@")
 

-                 or line.startswith("+")
 

-                 or line.startswith("-")
 

-             ):
 

-                 if line.startswith("@@"):
 

-                     output.append(
 

-                         '<tr class="stretch-table-column bg-light"\
 

-                     id="c-%(commit)s-%(cnt_lbl)s">'
 

-                         % ({"cnt_lbl": cnt, "commit": commit})
 

-                     )
 

-                 elif line.startswith("+"):
 

-                     output.append(
 

-                         '<tr class="stretch-table-column alert-success" \
 

-                     id="c-%(commit)s-%(cnt_lbl)s">'
 

-                         % ({"cnt_lbl": cnt, "commit": commit})
 

-                     )
 

-                 elif line.startswith("-"):
 

-                     output.append(
 

-                         '<tr class="stretch-table-column alert-danger" \
 

-                     id="c-%(commit)s-%(cnt_lbl)s">'
 

-                         % ({"cnt_lbl": cnt, "commit": commit})
 

-                     )
 

+         if line.startswith("@@"):
 

+             output.append(
 

+                 '<tr class="stretch-table-column bg-light"\
 

+             id="c-%(commit)s-%(cnt_lbl)s">'
 

+                 % ({"cnt_lbl": cnt, "commit": commit})
 

+             )
 

+             output.append(
 

+                 '<td class="cell1"></td><td class="prc border-right"></td>'
 

+             )
 

+         else:
 

+             if line.startswith("+"):
 

+                 output.append(
 

+                     '<tr class="stretch-table-column alert-success" \
 

+                 id="c-%(commit)s-%(cnt_lbl)s">'
 

+                     % ({"cnt_lbl": cnt, "commit": commit})
 

+                 )
 

+             elif line.startswith("-"):
 

+                 output.append(
 

+                     '<tr class="stretch-table-column alert-danger" \
 

+                 id="c-%(commit)s-%(cnt_lbl)s">'
 

+                     % ({"cnt_lbl": cnt, "commit": commit})
 

+                 )
 

              else:
 

                  output.append(
 

                      '<tr id="c-%(commit)s-%(cnt_lbl)s">'
 
@@ -211,13 +206,6 @@ 
 

                      }
 

                  )
 

              )
 

-         else:
 

-             output.append(
 

-                 '<tr><td class="cell1">'
 

-                 '<a id="%(cnt)s" href="#%(cnt)s" data-line-number='
 

-                 '"%(cnt_lbl)s"></a></td>'
 

-                 % ({"cnt": "%s_%s" % (index, cnt), "cnt_lbl": cnt})
 

-             )
 

  

          cnt += 1
 

          if not line:
 
@@ -254,29 +242,24 @@ 
 

                          + 'title="Open changed file"></span></a>'
 

                      )
 

  

-         if isprdiff and (
 

-             line.startswith("@@")
 

-             or line.startswith("+")
 

-             or line.startswith("-")
 

-         ):
 

-             if line.startswith("@@"):
 

-                 output.append(
 

-                     '<td class="cell2 stretch-table-column">\
 

-                     <pre class="text-muted"><code>%s</code></pre></td>'
 

-                     % line
 

-                 )
 

-             elif line.startswith("+"):
 

-                 output.append(
 

-                     '<td class="cell2 stretch-table-column">\
 

-                     <pre class="alert-success"><code>%s</code></pre></td>'
 

-                     % escape(line)
 

-                 )
 

-             elif line.startswith("-"):
 

-                 output.append(
 

-                     '<td class="cell2 stretch-table-column">\
 

-                     <pre class="alert-danger"><code>%s</code></pre></td>'
 

-                     % escape(line)
 

-                 )
 

+         if line.startswith("@@"):
 

+             output.append(
 

+                 '<td class="cell2 stretch-table-column">\
 

+                 <pre class="text-muted"><code>%s</code></pre></td>'
 

+                 % line
 

+             )
 

+         elif line.startswith("+"):
 

+             output.append(
 

+                 '<td class="cell2 stretch-table-column">\
 

+                 <pre class="alert-success"><code>%s</code></pre></td>'
 

+                 % escape(line)
 

+             )
 

+         elif line.startswith("-"):
 

+             output.append(
 

+                 '<td class="cell2 stretch-table-column">\
 

+                 <pre class="alert-danger"><code>%s</code></pre></td>'
 

+                 % escape(line)
 

+             )
 

          else:
 

              output.append(
 

                  '<td class="cell2"><pre><code>%s</code></pre></td>'

file modified
+22 -2 
@@ -302,11 +302,31 @@ 
 

          )
 

  

          self.assertIn(
 

-             '<span class="btn btn-success btn-sm font-weight-bold disabled opacity-100">+3</span>',
 

+             '<span class="btn btn-success btn-sm font-weight-bold disabled'
 

+             ' opacity-100">+3</span>',
 

              output_text,
 

          )
 

          self.assertIn(
 

-             '<span class="btn btn-danger btn-sm font-weight-bold disabled opacity-100">-1</span>',
 

+             '<span class="btn btn-danger btn-sm font-weight-bold disabled '
 

+             'opacity-100">-1</span>',
 

+             output_text,
 

+         )
 

+ 

+         # Test if hunk headline is rendered without line numbers
 

+         self.assertIn(
 

+             '<td class="cell1"></td><td class="prc border-right"></td>\n<td '
 

+             'class="cell2 stretch-table-column">                <pre class='
 

+             '"text-muted"><code>@@ -1,2 +1,4 @@',
 

+             output_text,
 

+         )
 

+         # Tests if line number 1 is displayed
 

+         self.assertNotIn(
 

+             '<td class="cell1"><a id="_1__1" href="#_1__1" data-line-number="1" data-file-number="1"></a></td>',
 

+             output_text,
 

+         )
 

+         # Test if line number 2 is displayed
 

+         self.assertIn(
 

+             '<td class="cell1"><a id="_1__2" href="#_1__2" data-line-number="2" data-file-number="1"></a></td>',
 

              output_text,
 

          )

Fixes #724

rebased onto b9a6f7868a93962888822793c9d9760d4106e989
4 years ago

:thumbsup:

Anyway we could test this?

Anyway we could test this?

Sure.

Should I remove the line numbers from the '@@' line too?

rebased onto ca304b8fd054f7de3fb7d0560392ea4a12812129
4 years ago

the format_loc() filter is used just on _diff_pull_request.html template, so I went with some cleaning on the way.

let see what folks and jenkins says about it :)

(I'll squash this)

diff.png
Edited 4 years ago by jlanda

Seems like tons of wrong assumptions on my side, i love it :D

rebased onto cbeb61ea50c2de795cfa707b218cbf138aa35796
4 years ago

5 new commits added

  • black formatting & tests
  • add line numbers on diff lines, but not on @@ line. remove add comment link on @@ line.
  • no need to check this on every line
  • we always have a valid index here
  • remove isprdiff option on format_loc filter
4 years ago

1 new commit added

  • fix black's dumbness
4 years ago

Now with tests! let me no if I have to squash them

1 new commit added

  • split long line
4 years ago

7 new commits added

  • split long lines
  • fix black's dumbness
  • black formatting & tests
  • add line numbers on diff lines, but not on @@ line. remove add comment link on @@ line.
  • no need to check this on every line
  • we always have a valid index here
  • remove isprdiff option on format_loc filter
4 years ago

@jlanda squashing would be appreciated :smile:

done =)

rebased onto 2a722ba5fe941202eb9e48964090b277e297b5a4
4 years ago

:thumbsup:

rebased onto 3bef580e2a562be2b88bf762f0490da1cbcd8867
4 years ago

rebased onto 8e3ae65
4 years ago

Tested locally and loving it!

Many many thanks!!

Pull-Request has been merged by pingou
4 years ago

Many many thanks!!

You're welcome :)
Metadata
None
No Tags
Flags
jenkins
success (100%)
Build #3645 successful (commit: 8e3ae652)
4 years ago
jenkins
success (100%)
Build #3644 successful (commit: 3bef580e)
4 years ago
jenkins
success (100%)
Build #3640 successful (commit: 2a722ba5)
4 years ago
jenkins
success (100%)
Build #3639 successful (commit: 5fb7c8ed)
4 years ago
jenkins
success (100%)
Build #3638 successful (commit: 5fb7c8ed)
4 years ago
jenkins
success (100%)
Build #3637 successful (commit: 5fb7c8ed)
4 years ago
jenkins
success (100%)
Build #3636 successful (commit: 5fb7c8ed)
4 years ago
jenkins
success (100%)
Build #3635 successful (commit: 9d860cb0)
4 years ago
jenkins
failure
Build #3634 failed (commit: 3c8b07ae)
4 years ago
jenkins
success (100%)
Build #3633 successful (commit: b9a6f786)
4 years ago
jenkins
failure
Build #3632 failed (commit: 5cf62252)
4 years ago
Changes Summary 3
+0 -1
file changed
pagure/templates/_repo_renderdiff.html
+42 -59
file changed
pagure/ui/filters.py
+22 -2
file changed
tests/test_pagure_flask_ui_fork.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.