Line filter can be extended 3 lines above changes and 3 lines below

[bwrsandman]

Take this change as an example

 MyClass::~MyClass()   # L1 -> L1
 {                     # L2 -> L2
-    delete m_Member1; # L3 -> x
-    delete m_Member2; # L4 -> x
-    delete m_Member3; # L5 -> x
 }                     # L6 -> L3

If I run clang tidy on it it will tell me that modernize-use-equals-default requires it be written as

MyClass::~MyClass() = default;

However, clang-tidy review's line filter will only include added lines when the change is required to be on L1. This means clang-tidy review will miss this fix.

In a github review, you are able to make a comment on 3 lines above and below a changed line before the option is no longer available.

My proposal is to modify get_line_ranges so that:

Added lines:

• each group range has its start decremented by 3 (min of 1)
• the size extended by 6 (max of line number in file - start - 1)
  Removed lines:
• Make a group that extends from the preceeding line (after change) to 2 above and 3 below

Care must be made to be sure to combine overlapping groups.

[ZedThree]

That does sound sensible, and that's a very compelling case in particular! If I understand correctly, we won't generate warnings "outside" of GitHub's PR diff view, but we might still generate warnings on untouched lines -- which would be wanted in this case, but perhaps not in others, for example:

 MyClass::~MyClass()   # L1 -> L1
 {                     # L2 -> L2
     delete m_Member1; # L3 -> L3
-    delete m_Member2; # L4 -> x   removing unused member
     delete m_Member3; # L5 -> L4
 }                     # L6 -> L5

We'd get warnings from L3 and L4 about raw pointers, even though this change doesn't touch them.

Maybe that's fine? Obviously people could put NOLINT comments on things that are necessary or not fixable right now, but it could make the action feel more "spammy".

[bwrsandman]

That's a good point I hadn't considered