Summary

Add typing for diff opcodes.

Review Request #14418 — Created May 2, 2025 and submitted June 10, 2025, 3:41 p.m.

Information

Owner

david

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This change updates diff opcodes to use explicitly typed tuples. The
opcodes created by the Differ implementation have a tag and ranges,
and then the opcode generator adds an additional metadata field. In the
interest of avoiding any changes, we therefore have separate definitions
for both steps.

Testing Done


Did a bunch of smoke testing of diff generation and saw that

  everything still worked correctly.
Ran unit tests.

Commits

Summary	ID
Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	f8ad232f1ebd19142135457de29d0b3156da9519

Summary

Add typing for diff opcodes.

This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.

f8ad232f1ebd19142135457de29d0b3156da9519

Issues

Description	From	Last Updated
'typing.Any' imported but unused Column: 1 Error code: F401	reviewbot	May 2, 2025, 11:10 p.m.
While here, can you put these in parens?	chipx86	May 23, 2025, 4:25 p.m.
Module path is missing a ..	chipx86	May 23, 2025, 4:25 p.m.
Can we put that back in alphabetical order?	chipx86	May 23, 2025, 4:25 p.m.
For this case, I'm pretty sure a tuple is actually faster. A set doesn't buy us anything. Especially constructed every …	chipx86	May 23, 2025, 3:23 p.m.
While we're cleaning this up, can we pull out groups, inserts, removes, and differ into local variables, so we don't …	chipx86	May 23, 2025, 4:26 p.m.
Same note about the tuple vs. set.	chipx86	May 23, 2025, 3:23 p.m.
Same as above.	chipx86	May 23, 2025, 3:23 p.m.
Same as above.	chipx86	May 23, 2025, 3:23 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/opcode_generator.py (Diff revision 1)
The issue has been dropped. Show all issues
```
'typing.Any' imported but unused

Column: 1
Error code: F401
```

Commits:

	Summary	ID
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	946fe9dc79d2726d896130feeb7fcbd2faf57c7d
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	6961dc9470b3655bda576024c5e1c517818f1aa3

Diff:

Revision 2 (+818 -266)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/differ.py
	reviewboard/diffviewer/myersdiff.py
	reviewboard/diffviewer/opcode_generator.py
	reviewboard/diffviewer/processors.py
	reviewboard/diffviewer/smdiff.py
	reviewboard/diffviewer/tests/test_myersdiff.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

	Summary	ID
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	6961dc9470b3655bda576024c5e1c517818f1aa3
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	f79f187458dcc42cf79371d2bc4670665d895ff8

Diff:

Revision 3 (+824 -266)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/differ.py
	reviewboard/diffviewer/myersdiff.py
	reviewboard/diffviewer/opcode_generator.py
	reviewboard/diffviewer/processors.py
	reviewboard/diffviewer/smdiff.py
	reviewboard/diffviewer/tests/test_myersdiff.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/myersdiff.py (Diff revision 3)
The issue has been resolved. Show all issues
```
While here, can you put these in parens?
```
reviewboard/diffviewer/opcode_generator.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Module path is missing a ..
```
reviewboard/diffviewer/opcode_generator.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Can we put that back in alphabetical order?
```

reviewboard/diffviewer/opcode_generator.py (Diff revision 3)

The issue has been dropped. Show all issues

For this case, I'm pretty sure a tuple is actually faster. A set doesn't buy us anything. Especially constructed every time we do the check.

david May 23, 2025, 3:24 p.m.

Ran a test on 4M checks, averaged across 20 runs. Sets are 13% faster. Both are extremely fast, though.

reviewboard/diffviewer/opcode_generator.py (Diff revision 3)

The issue has been resolved. Show all issues

While we're cleaning this up, can we pull out groups, inserts, removes, and differ into local variables, so we don't incur the lookup costs per-iteration?

reviewboard/diffviewer/opcode_generator.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Same note about the tuple vs. set.
```
reviewboard/diffviewer/processors.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Same as above.
```
reviewboard/diffviewer/processors.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Same as above.
```

Commits:

	Summary	ID
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	f79f187458dcc42cf79371d2bc4670665d895ff8
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	f8ad232f1ebd19142135457de29d0b3156da9519

Diff:

Revision 4 (+838 -270)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/differ.py
	reviewboard/diffviewer/myersdiff.py
	reviewboard/diffviewer/opcode_generator.py
	reviewboard/diffviewer/processors.py
	reviewboard/diffviewer/smdiff.py
	reviewboard/diffviewer/tests/test_myersdiff.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (df24e42)