-
-
-
reviewboard/diffviewer/diffutils.py (Diff revision 1) Col: 17 E128 continuation line under-indented for visual indent
-
reviewboard/diffviewer/diffutils.py (Diff revision 1) Col: 17 E128 continuation line under-indented for visual indent
-
-
reviewboard/diffviewer/diffutils.py (Diff revision 1) Col: 22 E128 continuation line under-indented for visual indent
Fix "List index out of range" errors with interdiffing deleted files.
Review Request #6473 — Created Oct. 20, 2014 and submitted
It's possible to hit an IndexError when processing an interdiff between two
changes that delete a file. The wrinkle here is that between the two revisions
of the diff, some additional changes to that file were made (and synced to the
local developer's tree).The result of this is that we didn't filter out the file from the list of
things to show for the interdiff, and then things went wrong when we tried to
process the opcodes for diffing two empty strings.While I was doing this, I saw some warnings about a unicode-unaware comparison
occurring, so I've slipped the fix for that in as well. This doesn't actually
have any correctness implications, it just speeds up things slightly when
dealing with empty files.
- Created a reproduction case for the bug and tested before and after the
change. Before, I saw an entry for this file with the error inside. After,
the deleted files don't show up at all in the interdiff. - Ran unit tests.
| Description | From | Last Updated |
|---|---|---|
|
Col: 17 E201 whitespace after '(' |
 reviewbot
|
|
|
Every Python linter complains about this format, and it's just not common in Python. Can we go back to the … |
 chipx86
|
|
|
Col: 17 E128 continuation line under-indented for visual indent |
reviewbot
|
|
|
Col: 17 E128 continuation line under-indented for visual indent |
reviewbot
|
|
|
Col: 22 E201 whitespace after '(' |
reviewbot
|
|
|
Col: 22 E128 continuation line under-indented for visual indent |
reviewbot
|
|
|
What happens for legitimately empty files? Do we want to do this just for interdiffs? Also, can we add a … |
chipx86
|
-
-
reviewboard/diffviewer/diffutils.py (Diff revision 1) Every Python linter complains about this format, and it's just not common in Python. Can we go back to the format we use everywhere else?
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 2 (+7 -5) |
-
Tool: Pyflakes Processed Files: reviewboard/diffviewer/diffutils.py Tool: PEP8 Style Checker Processed Files: reviewboard/diffviewer/diffutils.py
-
-
reviewboard/diffviewer/diffutils.py (Diff revision 2) Do we want/need to compare against
b'\r'here, or is this sufficient?
Change Summary:
Cover a couple more cases.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 3 (+12 -7) |
-
Tool: Pyflakes Processed Files: reviewboard/diffviewer/chunk_generator.py reviewboard/diffviewer/diffutils.py
-
-
reviewboard/diffviewer/chunk_generator.py (Diff revision 3) What happens for legitimately empty files?
Do we want to do this just for interdiffs?
Also, can we add a comment here detailing why we're skipping?
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 4 (+20 -7) |
-
Tool: Pyflakes Processed Files: reviewboard/diffviewer/chunk_generator.py reviewboard/diffviewer/diffutils.py Tool: PEP8 Style Checker Processed Files: reviewboard/diffviewer/chunk_generator.py reviewboard/diffviewer/diffutils.py
Change Summary:
Revert changes to chunk_generator.py
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 5 (+9 -7) |
Tool: PEP8 Style Checker Processed Files: reviewboard/diffviewer/diffutils.py Tool: Pyflakes Processed Files: reviewboard/diffviewer/diffutils.py