Modernize reviewboard.diffviewer.myersdiff.

Review Request #14504 — Created July 11, 2025 and updated

Information

Review Board
master

Reviewers

The Myers diff implementation is some of the oldest, most opaque code we
have. This change fixes things up as best I can.

Ran unit tests.

Summary ID
Modernize reviewboard.diffviewer.myersdiff.
The Myers diff implementation is some of the oldest, most opaque code we have. This change fixes things up as best I can. Testing Done: Ran unit tests.
9014eac91e2dc8f53c0f2c2bb704fe2d4d0265bb
Description From Last Updated

We access a_data and b_data 3 times, so let's just pull it out into a local variable. Not a hot …

chipx86chipx86

Let's pull these out into local variables, since we access these a lot.

chipx86chipx86

Can you add Args and Returns while here?

chipx86chipx86

Since we're accessing this every loop, let's pull ignore_space out.

chipx86chipx86

Let's also do the same with these and everything else we're accessing from self a lot.

chipx86chipx86

Small tweak: We could compare v > best first, since it avoids the math and calls, and might faster in …

chipx86chipx86

Let's pull a_data and b_data, out, since we use these a lot.

chipx86chipx86

"Check"

chipx86chipx86
chipx86
  1. This all looks great. Nice to see any cleanup in this file.

    There's some room for small performance gains while here. I've noted a few places where we can easily pull out attribute accesses to the same variables out of loops and other things.

  2. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     
     
    Show all issues

    We access a_data and b_data 3 times, so let's just pull it out into a local variable. Not a hot path, but any wins out of diff code would be good.

  3. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     
     
    Show all issues

    Let's pull these out into local variables, since we access these a lot.

  4. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     
     
     
     
     
    Show all issues

    Can you add Args and Returns while here?

  5. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     
    Show all issues

    Since we're accessing this every loop, let's pull ignore_space out.

  6. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     
     
     
    Show all issues

    Let's also do the same with these and everything else we're accessing from self a lot.

  7. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     
     
    Show all issues

    Small tweak: We could compare v > best first, since it avoids the math and calls, and might faster in the false case.

  8. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     
     
    Show all issues

    Let's pull a_data and b_data, out, since we use these a lot.

  9. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     

    Hmm, / vs. // changed and we never updated this, so I know we're now going back to the original behavior, but I wonder if we'll see any subtle changes from this

    1. I don't think so. All our tests are unchanged.

  10. reviewboard/diffviewer/myersdiff.py (Diff revision 1)
     
     
    Show all issues

    "Check"

  11. 
      
david
Review request changed
Commits:
Summary ID
Modernize reviewboard.diffviewer.myersdiff.
The Myers diff implementation is some of the oldest, most opaque code we have. This change fixes things up as best I can. Testing Done: Ran unit tests.
de44b56e96ff89ca516da7b35474e6a9963ba271
Modernize reviewboard.diffviewer.myersdiff.
The Myers diff implementation is some of the oldest, most opaque code we have. This change fixes things up as best I can. Testing Done: Ran unit tests.
9014eac91e2dc8f53c0f2c2bb704fe2d4d0265bb

Checks run (2 succeeded)

flake8 passed.
JSHint passed.