Reduce attribute access and use kwarg-based calls for some diff logic.

Review Request #10770 — Created Oct. 29, 2019 and updated

Review Board

This is a cleanup change that improves readability and reduces attribute
access for some diff viewer logic, in preparation for some larger
changes coming soon to that code.

DiffChunkGenerator.get_chunks_uncached() had a large amount of
redundant attribute lookups when calculating state needed to build the
chunks. This was unnecessary, and just made the code a little more
noisy and technically slower than needed. We now pull in a few of these
variables into locals at the start of the method.

We also had a lot of calls to get_original_file(),
get_original_file_from_repo(), get_patched_file(), patch(), and
the PatchError constructor that were using positional arguments. Some
of these function signatures are going to get some changes soon, so to
help prepare for that and to make the code more self-documenting, this
change also updates these calls to pass arguments as keywords.

This uncovered some incorrect (but otherwise harmless) calls in some of
our unit tests for patching that would have been avoided if using
keyword arguments to begin with.

Unit tests pass.

Tested that diffs and interdiffs work correctly.

Checks run (2 succeeded)
flake8 passed.
JSHint passed.
  1. Ship It!