Sort files in the diff intelligently
Updated 1 year, 5 months ago
| Christian Hammond | Reviewers | ||
| trunk | reviewboard | ||
| 159 | |||
| None | Review Board SVN | ||
Sorts files in the diff when a diff is being uploaded. The files are alphabetical, except we put header files before implementation files in order to ease reviews and give reviewers an overview of the structure before seeing the implementation. We do this at diff creation time instead of view time. This means that existing diffs will retain their sorting, which is actually important because files contain index-based anchors and we don't want to break existing links.
Created a diff with several .c and .h files and saw that the order was as expected: alphabetical, but with .h files before .c files.
Diff revision 1 (Latest)
- /trunk/reviewboard/diffviewer/forms.py: 3 changes [ 1 2 3 ]
Expand changes| /trunk/reviewboard/diffviewer/forms.py | |||
|---|---|---|---|
| Revision 814 | New Change | ||
| ... | 16 lines hidden [Expand] | ||
| 17 | # XXX: it'd be really nice to have "required" for these things be |
17 | # XXX: it'd be really nice to have "required" for these things be |
| 18 | # dependent on the scmtool for the repository |
18 | # dependent on the scmtool for the repository |
| 19 | basedir = forms.CharField(required=False) |
19 | basedir = forms.CharField(required=False) |
| 20 | path = forms.CharField(widget=forms.FileInput()) |
20 | path = forms.CharField(widget=forms.FileInput()) |
| 21 | 21 | ||
| 22 | # Extensions used for intelligent sorting of header files |
||
| 23 | # before implementation files. |
||
| 24 | HEADER_EXTENSIONS = ["h", "H", "hh", "hpp", "hxx", "h++"] |
||
| 25 | IMPL_EXTENSIONS = ["c", "C", "cc", "cpp", "cxx", "c++", "m", "mm", "M"] |
||
| 26 | |||
| 22 | def create(self, file, diffset_history=None): |
27 | def create(self, file, diffset_history=None): |
| 23 | # Parse the diff |
28 | # Parse the diff |
| 24 | repository = Repository.objects.get(pk=self.cleaned_data['repositoryid']) |
29 | repository = Repository.objects.get(pk=self.cleaned_data['repositoryid']) |
| 25 | tool = repository.get_scmtool() |
30 | tool = repository.get_scmtool() |
| 26 | files = tool.getParser(file["content"]).parse() |
31 | files = tool.getParser(file["content"]).parse() |
| ... | 23 lines hidden [Expand] | ||
| 50 | history=diffset_history, |
55 | history=diffset_history, |
| 51 | diffcompat=DEFAULT_DIFF_COMPAT_VERSION) |
56 | diffcompat=DEFAULT_DIFF_COMPAT_VERSION) |
| 52 | diffset.repository = repository |
57 | diffset.repository = repository |
| 53 | diffset.save() |
58 | diffset.save() |
| 54 | 59 | ||
| 60 | # Sort the files so that header files come before implementation. |
||
| 61 | files.sort(cmp=self._compare_files, key=lambda f: f.origFile) |
||
| 62 | |||
| 55 | for f in files: |
63 | for f in files: |
| 56 | filediff = FileDiff(diffset=diffset, |
64 | filediff = FileDiff(diffset=diffset, |
| 57 | source_file=f.origFile, |
65 | source_file=f.origFile, |
| 58 | dest_file=os.path.join(basedir, f.newFile), |
66 | dest_file=os.path.join(basedir, f.newFile), |
| 59 | source_revision=str(f.origInfo), |
67 | source_revision=str(f.origInfo), |
| 60 | dest_detail=f.newInfo, |
68 | dest_detail=f.newInfo, |
| 61 | diff=f.data, |
69 | diff=f.data, |
| 62 | binary=f.binary) |
70 | binary=f.binary) |
| 63 | filediff.save() |
71 | filediff.save() |
| 64 | 72 | ||
| 65 | return diffset |
73 | return diffset |
| 74 | |||
| 75 | def _compare_files(self, filename1, filename2): |
||
| 76 | """ |
||
| 77 | Compares two files, giving precedence to header files over source |
||
| 78 | files. This allows the resulting list of files to be more |
||
| 79 | intelligently sorted. |
||
| 80 | """ |
||
| 81 | if filename1.find('.') != -1 and filename2.find('.') != -1: |
||
| 82 | basename1, ext1 = filename1.rsplit('.') |
||
| 83 | basename2, ext2 = filename2.rsplit('.') |
||
| 84 | |||
| 85 | if basename1 == basename2: |
||
| 86 | if ext1 in self.HEADER_EXTENSIONS and \ |
||
| 87 | ext2 in self.IMPL_EXTENSIONS: |
||
| 88 | return -1 |
||
| 89 | elif ext1 in self.IMPL_EXTENSIONS and \ |
||
| 90 | ext2 in self.HEADER_EXTENSIONS: |
||
| 91 | return 1 |
||
| 92 | |||
| 93 | return cmp(filename1, filename2) |
||
- /trunk/reviewboard/diffviewer/forms.py: 3 changes [ 1 2 3 ]
Other reviews