Summary

Split DiffChunkGenerator into a general-purpose RawDiffChunkGenerator.

Review Request #7093 — Created March 20, 2015 and submitted March 25, 2015, 7:56 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-2.5.x

Bugs

Depends On

Commit

3b0ad74...

Reviewers

Groups

reviewboard

People

Description

DiffChunkGenerator is a very useful and powerful class for producing a

set of chunks we can directly use for rendering diffs. To use it, it

required the whole diff machinery: FileDiff models, DiffSet,

repositories, the patching process, etc. That made it impractical for

any usage beyond showing diffs of files in a repository.
This change splits the heavy processing out into a RawDiffChunkGenerator

class, which can be used to generate chunks from any two strings.
DiffChunkGenerator subclasses RawDiffChunkGenerator, doing all the work

of handling interdiffs and generating suitable content from FileDiffs to

pass down to the RawDiffChunkGenerator functions.
Along with this, DiffOpcodeGenerator no longer needs FileDiffs as well.

It only ever needed the diff content stored on them, for interdiff

processing, so we just pass that directly in DiffChunkGenerator.

Testing Done

Browsed diffs and interdiffs, comparing them to versions without the path.
This covered all types of changes, including indentation. I didn't see any
differences in behavior.

Tested with raw strings, and got chunks out of them.

Unit tests pass.

Issues

Description	From	Last Updated
Col: 1 E303 too many blank lines (3)	reviewbot	March 20, 2015, 2:03 a.m.
'os' imported but unused	reviewbot	March 20, 2015, 2:03 a.m.
"Generate"	brennie	March 25, 2015, 5:58 p.m.
I know this isn't in the change, but aren't there also "replace" chunks?	brennie	March 20, 2015, 4:34 p.m.
You should use encoding_list=None and assign encoding_list to [] if it is None in the function.	brennie	March 20, 2015, 4:35 p.m.
"Generate"	brennie	March 25, 2015, 5:58 p.m.
Again, replace sections?	brennie	March 20, 2015, 4:35 p.m.
"Create"	brennie	March 20, 2015, 4:35 p.m.
Should be "Return".	brennie	March 20, 2015, 4:35 p.m.
Line needs b''	brennie	March 20, 2015, 4:35 p.m.
Blank line between these.	brennie	March 20, 2015, 4:35 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/opcode_generator.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/opcode_generator.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py

reviewboard/diffviewer/chunk_generator.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 E303 too many blank lines (3)
```
reviewboard/diffviewer/tests.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'os' imported but unused
```

Change Summary:

Fixed ReviewBot complaints.

Commit:

22d54eb4bab793b8871a4a74b8c3617bdb614ea2

b3996d6ecdbcb3f929ce584b21dba22c2514f5ca

Diff:

Revision 2 (+354 -220)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/opcode_generator.py
	reviewboard/diffviewer/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/opcode_generator.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/opcode_generator.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py

reviewboard/diffviewer/chunk_generator.py (Diff revision 2)

The issue has been dropped. Show all issues

"Generate"

chipx86 March 20, 2015, 4:36 p.m.

IIRC, we only use that mood for functions, not classes. I'm not sure where it even says anything about this in the PEP though. Should get David's opinion.

reviewboard/diffviewer/chunk_generator.py (Diff revision 2)
The issue has been resolved. Show all issues
```
I know this isn't in the change, but aren't there also "replace" chunks?
```

reviewboard/diffviewer/chunk_generator.py (Diff revision 2)

The issue has been resolved. Show all issues

You should use encoding_list=None and assign encoding_list to [] if it is None in the function.

reviewboard/diffviewer/chunk_generator.py (Diff revision 2)
The issue has been dropped. Show all issues
```
"Generate"
```
reviewboard/diffviewer/chunk_generator.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Again, replace sections?
```
reviewboard/diffviewer/chunk_generator.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"Create"
```
reviewboard/diffviewer/chunk_generator.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Should be "Return".
```
reviewboard/diffviewer/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Line needs b''
```
reviewboard/diffviewer/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Blank line between these.
```

Change Summary:


Changed some docstrings.
Fixed some defaults.
Added some missing blank lines.

Commit:

b3996d6ecdbcb3f929ce584b21dba22c2514f5ca

3b0ad74ab108c5e8ca5319c720942287820cb5a7

Diff:

Revision 3 (+358 -223)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/opcode_generator.py
	reviewboard/diffviewer/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/opcode_generator.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/opcode_generator.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/chunk_generator.py
    reviewboard/diffviewer/diffutils.py

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-2.5.x (c246d7a)