diff --git a/reviewboard/diffviewer/diffutils.py b/reviewboard/diffviewer/diffutils.py
index 4da4c877a123ddf8e81e05333617b323420ba900..b4f77e209ac2e813d3f15a225471fc901c59be85 100644
--- a/reviewboard/diffviewer/diffutils.py
+++ b/reviewboard/diffviewer/diffutils.py
@@ -10,15 +10,14 @@ import shutil
import subprocess
import tempfile
from difflib import SequenceMatcher
-from functools import cmp_to_key
-from typing import Any, AnyStr, Iterator, Optional, TYPE_CHECKING
+from typing import (Any, AnyStr, Callable, Iterator, Optional, Sequence,
+ TYPE_CHECKING, TypeVar)
from django.core.files.base import ContentFile
from django.utils.encoding import force_str
from django.utils.translation import gettext as _
from djblets.log import log_timed
from djblets.siteconfig.models import SiteConfiguration
-from djblets.util.compat.python.past import cmp
from djblets.util.contextmanagers import controlled_subprocess
from housekeeping.functions import deprecate_non_keyword_only_args
from typing_extensions import NotRequired, TypedDict
@@ -27,6 +26,8 @@ from reviewboard.attachments.mimetypes import guess_mimetype
from reviewboard.deprecation import RemovedInReviewBoard80Warning
from reviewboard.diffviewer.commit_utils import exclude_ancestor_filediffs
from reviewboard.diffviewer.errors import DiffTooBigError, PatchError
+from reviewboard.diffviewer.filetypes import (HEADER_EXTENSIONS,
+ IMPL_EXTENSIONS)
from reviewboard.diffviewer.settings import DiffSettings
from reviewboard.scmtools.core import FileLookupContext, PRE_CREATION, HEAD
@@ -59,6 +60,9 @@ NEWLINE_UNICODE_RE = re.compile(r'(?:\n|\r(?:\r?\n)?)')
_PATCH_GARBAGE_INPUT = 'patch: **** Only garbage was found in the patch input.'
+_T = TypeVar('_T')
+
+
class SerializedDiffFile(TypedDict):
"""Serialized information on a diff file.
@@ -2084,8 +2088,11 @@ def get_line_changed_regions(oldline, newline):
return oldchanges, newchanges
-def get_sorted_filediffs(filediffs, key=None):
- """Sorts a list of filediffs.
+def get_sorted_filediffs(
+ filediffs: Sequence[_T],
+ key: Optional[Callable[[_T], FileDiff]] = None,
+) -> Sequence[_T]:
+ """Sorts a list of filediffs or filediff-wrapping objects.
The list of filediffs will be sorted first by their base paths in
ascending order.
@@ -2101,44 +2108,56 @@ def get_sorted_filediffs(filediffs, key=None):
must provide a callable ``key`` parameter that will return a FileDiff
for the given entry in the list. This will only be called once per
item.
+
+ Args:
+ filediffs (list of object):
+ The list of FileDiffs (or objects to convert to FileDiffs) to
+ sort.
+
+ key (callable, optional):
+ The optional function used to transform sequences of items to
+ FileDiffs.
+
+ Returns:
+ list of reviewboard.diffviewer.models.FileDiff:
+ The sorted list of filediffs.
"""
- def cmp_filediffs(filediff1, filediff2):
- x = make_key(filediff1)
- y = make_key(filediff2)
-
- # Sort based on basepath in ascending order.
- if x[0] != y[0]:
- a = x[0]
- b = y[0]
- else:
- # Sort based on filename in ascending order, then based on
- # the extension in descending order, to make *.h sort ahead of
- # *.c/cpp.
- x_file, x_ext = os.path.splitext(x[1])
- y_file, y_ext = os.path.splitext(y[1])
-
- if x_file == y_file:
- a = y_ext
- b = x_ext
- else:
- a = x_file
- b = y_file
+ from reviewboard.diffviewer.models import FileDiff
- return cmp(a, b)
+ def make_key(
+ item: _T,
+ ) -> tuple[str, str, int, str]:
+ filediff: FileDiff
- def make_key(filediff):
if key:
- filediff = key(filediff)
+ filediff = key(item)
+ else:
+ assert isinstance(item, FileDiff)
+
+ filediff = item
filename = filediff.dest_file
i = filename.rfind('/')
if i == -1:
- return '', filename
+ base_path = ''
else:
- return filename[:i], filename[i + 1:]
+ base_path = filename[:i]
+ filename = filename[i + 1:]
+
+ base_name, ext = os.path.splitext(filename)
+ ext = ext[1:]
+
+ if ext in HEADER_EXTENSIONS:
+ order_flag = 1
+ elif ext in IMPL_EXTENSIONS:
+ order_flag = 2
+ else:
+ order_flag = 0
+
+ return base_path, base_name, order_flag, ext
- return sorted(filediffs, key=cmp_to_key(cmp_filediffs))
+ return sorted(filediffs, key=make_key)
def get_displayed_diff_line_ranges(chunks, first_vlinenum, last_vlinenum):
diff --git a/reviewboard/diffviewer/filediff_creator.py b/reviewboard/diffviewer/filediff_creator.py
index e060a0916b16683bd8b3d1e7b6778d0d3504fbe4..7e85007e06aa1222a49a2d551d8a132196f48de8 100644
--- a/reviewboard/diffviewer/filediff_creator.py
+++ b/reviewboard/diffviewer/filediff_creator.py
@@ -2,11 +2,9 @@
import os
from copy import deepcopy
-from functools import cmp_to_key
from django.utils.encoding import force_bytes, force_str
from django.utils.translation import gettext as _
-from djblets.util.compat.python.past import cmp
from reviewboard.diffviewer.errors import EmptyDiffError
from reviewboard.scmtools.core import (FileLookupContext,
@@ -16,17 +14,6 @@ from reviewboard.scmtools.core import (FileLookupContext,
from reviewboard.scmtools.errors import FileNotFoundError
-# Extensions used for intelligent sorting of header files
-# before implementation files.
-_HEADER_EXTENSIONS = [
- b'h', b'H', b'hh', b'hpp', b'hxx', b'h++'
-]
-
-_IMPL_EXTENSIONS = [
- b'c', b'C', b'cc', b'cpp', b'cxx', b'c++', b'm', b'mm', b'M'
-]
-
-
def create_filediffs(diff_file_contents, parent_diff_file_contents,
repository, basedir, base_commit_id, diffset,
request=None, check_existence=True, get_file_exists=None,
@@ -369,10 +356,6 @@ def _prepare_diff_info(diff_file_contents, parent_diff_file_contents,
if len(files) == 0:
raise EmptyDiffError(_('The diff is empty.'))
- # Sort the files so that header files come before implementation
- # files.
- files.sort(key=cmp_to_key(_compare_files))
-
parsed_parent_diff = None
parent_files = {}
@@ -543,44 +526,6 @@ def _process_files(parsed_diff, basedir, repository, base_commit_id,
yield f
-def _compare_files(file1, file2):
- """Compare two files to determine a relative sort order.
-
- This will compare two files, giving precedence to header files over
- source files. This allows the resulting list of files to be more
- intelligently sorted.
-
- Args:
- file1 (reviewboard.diffviewer.parser.ParsedDiffFile):
- The first file to compare.
-
- file2 (reviewboard.diffviewer.parser.ParsedDiffFile):
- The second file to compare.
-
- Returns:
- int:
- -1 if ``file1`` should appear before ``file2``.
-
- 0 if ``file1`` and ``file2`` are considered equal.
-
- 1 if ``file1`` should appear after ``file2``.
- """
- filename1 = file1.orig_filename
- filename2 = file2.orig_filename
-
- if filename1.find(b'.') != -1 and filename2.find(b'.') != -1:
- basename1, ext1 = filename1.rsplit(b'.', 1)
- basename2, ext2 = filename2.rsplit(b'.', 1)
-
- if basename1 == basename2:
- if (ext1 in _HEADER_EXTENSIONS and ext2 in _IMPL_EXTENSIONS):
- return -1
- elif (ext1 in _IMPL_EXTENSIONS and ext2 in _HEADER_EXTENSIONS):
- return 1
-
- return cmp(filename1, filename2)
-
-
def _normalize_filename(filename, basedir):
"""Normalize a filename to be relative to the repository root.
diff --git a/reviewboard/diffviewer/filetypes.py b/reviewboard/diffviewer/filetypes.py
index a235dbeb101bd230b7fbf7e374a80cfdd80a06ce..69631fc0dad76bd8a0d10269aae258c0827d11d5 100644
--- a/reviewboard/diffviewer/filetypes.py
+++ b/reviewboard/diffviewer/filetypes.py
@@ -1,6 +1,41 @@
import re
+#: Common extensions for source code header files.
+#:
+#: This is largely geared toward C-like languages.
+#:
+#: Version Added:
+#: 7.0.2
+HEADER_EXTENSIONS = {
+ 'h',
+ 'H',
+ 'hh',
+ 'hpp',
+ 'hxx',
+ 'h++',
+}
+
+
+#: Common extensions for source code implementation files.
+#:
+#: This is largely geared toward C-like languages.
+#:
+#: Version Added:
+#: 7.0.2
+IMPL_EXTENSIONS = {
+ 'c',
+ 'C',
+ 'cc',
+ 'cpp',
+ 'cxx',
+ 'c++',
+ 'm',
+ 'mm',
+ 'M',
+}
+
+
# A list of regular expressions for headers in the source code that we can
# display in collapsed regions of diffs and diff fragments in reviews.
HEADER_REGEXES = {
diff --git a/reviewboard/diffviewer/tests/test_diffutils.py b/reviewboard/diffviewer/tests/test_diffutils.py
index 34dea5992063ac23f1ed462d9027beb6e2c666c3..cd1fa17ed2cb28bcbc6518b4986c7a9ad48fc725 100644
--- a/reviewboard/diffviewer/tests/test_diffutils.py
+++ b/reviewboard/diffviewer/tests/test_diffutils.py
@@ -29,6 +29,8 @@ from reviewboard.diffviewer.diffutils import (
split_line_endings,
_PATCH_GARBAGE_INPUT,
_get_last_header_in_chunks_before_line)
+from reviewboard.diffviewer.filetypes import (HEADER_EXTENSIONS,
+ IMPL_EXTENSIONS)
from reviewboard.diffviewer.errors import PatchError
from reviewboard.diffviewer.models import DiffCommit, FileDiff
from reviewboard.diffviewer.settings import DiffSettings
@@ -2439,6 +2441,118 @@ class GetMatchedInterdiffFilesTests(TestCase):
])
+class GetSortedFileDiffsTests(TestCase):
+ """Unit tests for get_sorted_filediffs.
+
+ Version Added:
+ 7.0
+ """
+
+ fixtures = [
+ 'test_users',
+ ] + BaseFileDiffAncestorTests.fixtures
+
+ def test_get_sorted_filediffs(self) -> None:
+ """Testing get_sorted_filediffs"""
+ repository = self.create_repository(tool_name='Test')
+ review_request = self.create_review_request(repository=repository)
+ diffset = self.create_diffset(review_request=review_request)
+
+ filediffs = [
+ self.create_filediff(
+ diffset=diffset,
+ source_file='zzz',
+ dest_file='zzz',
+ source_revision='1',
+ diff=b''),
+ ]
+
+ for impl_filename in sorted(IMPL_EXTENSIONS, reverse=True):
+ filediffs += [
+ self.create_filediff(
+ diffset=diffset,
+ source_file=f'/src/inner/file.{impl_filename}',
+ dest_file=f'/src/inner/file.{impl_filename}',
+ source_revision='1',
+ diff=b''),
+ self.create_filediff(
+ diffset=diffset,
+ source_file=f'file.{impl_filename}',
+ dest_file=f'file.{impl_filename}',
+ source_revision='1',
+ diff=b''),
+ self.create_filediff(
+ diffset=diffset,
+ source_file=f'/src/file.{impl_filename}',
+ dest_file=f'/src/file.{impl_filename}',
+ source_revision='1',
+ diff=b''),
+ ]
+
+ for header_filename in sorted(HEADER_EXTENSIONS, reverse=True):
+ filediffs += [
+ self.create_filediff(
+ diffset=diffset,
+ source_file=f'/src/inner/file.{header_filename}',
+ dest_file=f'/src/inner/file.{header_filename}',
+ source_revision='1',
+ diff=b''),
+ self.create_filediff(
+ diffset=diffset,
+ source_file=f'file.{header_filename}',
+ dest_file=f'file.{header_filename}',
+ source_revision='1',
+ diff=b''),
+ self.create_filediff(
+ diffset=diffset,
+ source_file=f'/src/file.{header_filename}',
+ dest_file=f'/src/file.{header_filename}',
+ source_revision='1',
+ diff=b''),
+ ]
+
+ filediffs.append(self.create_filediff(
+ diffset=diffset,
+ source_file='aaa',
+ dest_file='aaa',
+ source_revision='1',
+ diff=b''))
+
+ self.assertEqual(
+ [
+ filediff.source_file
+ for filediff in get_sorted_filediffs(filediffs)
+ ],
+ [
+ 'aaa',
+ *[
+ f'file.{filename}'
+ for filename in sorted(HEADER_EXTENSIONS)
+ ],
+ *[
+ f'file.{filename}'
+ for filename in sorted(IMPL_EXTENSIONS)
+ ],
+ 'zzz',
+ *[
+ f'/src/file.{filename}'
+ for filename in sorted(HEADER_EXTENSIONS)
+ ],
+ *[
+ f'/src/file.{filename}'
+ for filename in sorted(IMPL_EXTENSIONS)
+ ],
+ *[
+ f'/src/inner/file.{filename}'
+ for filename in sorted(HEADER_EXTENSIONS)
+ ],
+ *[
+ f'/src/inner/file.{filename}'
+ for filename in sorted(IMPL_EXTENSIONS)
+ ],
+ ])
+
+
class GetLineChangedRegionsTests(TestCase):
"""Unit tests for get_line_changed_regions."""
diff --git a/reviewboard/reviews/tests/test_comment_diff_fragments_view.py b/reviewboard/reviews/tests/test_comment_diff_fragments_view.py
index 2b8b3cb7fb8ee84e1a94083da71dca912c27f76c..5ff01b9f5b15ea126abc774470e651d5edf394b0 100644
--- a/reviewboard/reviews/tests/test_comment_diff_fragments_view.py
+++ b/reviewboard/reviews/tests/test_comment_diff_fragments_view.py
@@ -449,7 +449,7 @@ class CommentDiffFragmentsViewTests(BaseFileDiffAncestorTests):
# proxy to ensure that we're rendering with the correct commit
# information.
self.assertIn(
- 'bar',
+ 'bar',
html)
def test_comment_fragment_on_commit_range2(self) -> None:
@@ -469,7 +469,7 @@ class CommentDiffFragmentsViewTests(BaseFileDiffAncestorTests):
# information.
self.assertIn(
'bar',
+ 'tip-commit-id=3#file7line1">bar',
html)
def test_diff_comment_links_with_commits(self) -> None:
@@ -482,11 +482,11 @@ class CommentDiffFragmentsViewTests(BaseFileDiffAncestorTests):
self.assertEqual(
self.commit_comment1.get_absolute_url(),
- '/r/1/diff/1/?tip-commit-id=3#file6line1')
+ '/r/1/diff/1/?tip-commit-id=3#file7line1')
self.assertEqual(
self.commit_comment2.get_absolute_url(),
- '/r/1/diff/1/?base-commit-id=2&tip-commit-id=3#file6line1')
+ '/r/1/diff/1/?base-commit-id=2&tip-commit-id=3#file7line1')
def _create_commit_comment_data(self) -> None:
"""Create the test data for commit comments."""