Skip to content

Commit 5948b96

Browse files
fix: Correct --context-lines behavior for non-line-specific comments
This commit fixes an issue in `scripts/gha/get_pr_review_comments.py` where the `--context-lines` argument did not correctly suppress the full diff hunk for comments not associated with a specific line (i.e., where the API 'position' field is null). The `print_contextual_diff_hunk` function has been updated to: - Print an explanatory message instead of the full diff hunk when `--context-lines > 0` and the comment's position is null or invalid. - Retain the behavior of printing the full diff hunk if `--context-lines 0` is specified. - A redundant line in the context calculation logic was also removed. This ensures that setting a context limit via `--context-lines` will not unexpectedly display full diff hunks for file-level or other non-line-specific comments.
1 parent e84b02d commit 5948b96

File tree

1 file changed

+18
-12
lines changed

1 file changed

+18
-12
lines changed

scripts/gha/get_pr_review_comments.py

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,34 +30,40 @@
3030

3131

3232
def print_contextual_diff_hunk(diff_hunk, comment_position, context_lines_count):
33-
if not diff_hunk or not diff_hunk.strip(): # Handle empty or whitespace-only diff_hunk
33+
if not diff_hunk or not diff_hunk.strip():
3434
print("(No diff hunk available or content is empty)")
3535
return
3636

37-
hunk_lines = diff_hunk.split('\n')
37+
hunk_lines = diff_hunk.split('\n') # Note: Python's split('\n') is generally fine.
3838

39-
# comment_position is 1-indexed from GitHub API. If None, or context is 0, print full hunk.
40-
if context_lines_count == 0 or comment_position is None or comment_position < 1 or comment_position > len(hunk_lines):
39+
# Case 1: User explicitly wants the full hunk
40+
if context_lines_count == 0:
4141
print(diff_hunk)
4242
return
4343

44-
comment_line_index = comment_position - 1 # Convert to 0-indexed for list access
44+
# Case 2: Contextual display is requested (context_lines_count > 0),
45+
# but comment is not on a specific line or position is invalid for contextual display.
46+
if comment_position is None or comment_position < 1 or comment_position > len(hunk_lines):
47+
print("(Comment is not on a specific line in the diff, or position is invalid; full hunk context suppressed by --context-lines setting)")
48+
# As an alternative to the above message, if the hunk is small, one might choose to print it.
49+
# However, sticking to the user's feedback of not wanting full hunks when context is specified:
50+
# print(diff_hunk) # This would be the old behavior for this case.
51+
return
52+
53+
# Case 3: Contextual display is possible and requested
54+
comment_line_index = comment_position - 1 # Convert to 0-indexed
4555

4656
start_index = max(0, comment_line_index - context_lines_count)
4757
end_index = min(len(hunk_lines), comment_line_index + context_lines_count + 1)
4858

49-
# Ensure start_index is not greater than comment_line_index, in case of small hunks
50-
# This also means that if comment_line_index is valid, start_index will be <= comment_line_index
51-
start_index = min(start_index, comment_line_index if comment_line_index >=0 else 0)
52-
59+
# The following line was identified as redundant and is removed:
60+
# start_index = min(start_index, comment_line_index if comment_line_index >=0 else 0)
5361

5462
for i in range(start_index, end_index):
55-
# Basic safety for i, though start/end logic should make this robust
63+
# Robust check, though start/end logic should prevent out-of-bounds
5664
if i >= 0 and i < len(hunk_lines):
5765
prefix = "> " if i == comment_line_index else " "
5866
print(f"{prefix}{hunk_lines[i]}")
59-
# else: # This case should ideally not be reached with correct boundary conditions
60-
# print(f" Error: Skipped line index {i} in hunk processing due to boundary issue.")
6167

6268

6369
def main():

0 commit comments

Comments
 (0)