Skip to content

Commit 5a4010f

Browse files
feat: Improve context display and suggested command robustness
This commit enhances `scripts/gha/get_pr_review_comments.py` in two ways: 1. Suggested Command: The "suggest next command" feature now prepends `sys.executable` to the command. This ensures that the suggested command uses the same Python interpreter that the script was originally run with, making it more robust across different environments or if a specific interpreter was used. 2. Diff Hunk Context Display: - The default for `--context-lines` is now 10 (reverted from 0). - When `--context-lines > 0`, the script will first print the diff hunk header line (if it starts with "@@ "). - It will then print the last N (`args.context_lines`) lines from the *remainder* of the hunk. This ensures the header is shown for context, and then the trailing lines of that hunk are displayed, avoiding double-printing of the header if it would have naturally fallen into the "last N lines" of the full hunk. - If `--context-lines == 0`, the full hunk is displayed.
1 parent b900c7f commit 5a4010f

File tree

1 file changed

+33
-23
lines changed

1 file changed

+33
-23
lines changed

scripts/gha/get_pr_review_comments.py

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232
pass # firebase_github.py uses absl.logging.info, so this won't redirect.
3333

3434
def main():
35+
STATUS_IRRELEVANT = "[IRRELEVANT]"
36+
STATUS_OLD = "[OLD]"
37+
STATUS_CURRENT = "[CURRENT]"
38+
3539
default_owner = firebase_github.OWNER
3640
default_repo = firebase_github.REPO
3741

@@ -66,9 +70,8 @@ def main():
6670
parser.add_argument(
6771
"--context-lines",
6872
type=int,
69-
default=0,
70-
help="Number of context lines from the diff hunk. Use 0 for the full hunk. "
71-
"If > 0, shows the last N lines of the hunk. Default: 0."
73+
default=10,
74+
help="Number of context lines from the diff hunk. 0 for full hunk. If > 0, shows header (if any) and last N lines of the remaining hunk. Default: 10."
7275
)
7376
parser.add_argument(
7477
"--since",
@@ -133,22 +136,21 @@ def main():
133136
# is_effectively_outdated is no longer needed with the new distinct flags
134137

135138
if current_pos is None:
136-
status_text = "[IRRELEVANT]"
139+
status_text = STATUS_IRRELEVANT
137140
line_to_display = original_line
138141
elif original_line is not None and current_line != original_line:
139-
status_text = "[OLD]"
142+
status_text = STATUS_OLD
140143
line_to_display = current_line
141144
else:
142-
status_text = "[CURRENT]"
145+
status_text = STATUS_CURRENT
143146
line_to_display = current_line
144147

145148
if line_to_display is None:
146149
line_to_display = "N/A"
147150

148-
# New filtering logic
149-
if status_text == "[IRRELEVANT]" and not args.include_irrelevant:
151+
if status_text == STATUS_IRRELEVANT and not args.include_irrelevant:
150152
continue
151-
if status_text == "[OLD]" and args.exclude_old:
153+
if status_text == STATUS_OLD and args.exclude_old:
152154
continue
153155

154156
# Update latest timestamp (only for comments that will be printed)
@@ -190,15 +192,25 @@ def main():
190192
print("\n### Context:")
191193
print("```") # Start of Markdown code block
192194
if diff_hunk and diff_hunk.strip():
193-
hunk_lines = diff_hunk.split('\n')
194195
if args.context_lines == 0: # User wants the full hunk
195196
print(diff_hunk)
196-
elif args.context_lines > 0: # User wants N lines of context (last N lines)
197-
lines_to_print_count = args.context_lines
198-
actual_lines_to_print = hunk_lines[-lines_to_print_count:]
199-
for line_content in actual_lines_to_print:
200-
print(line_content)
201-
else:
197+
else: # User wants N lines of context (args.context_lines > 0)
198+
hunk_lines = diff_hunk.split('\n')
199+
header_printed_this_time = False
200+
if hunk_lines and hunk_lines[0].startswith("@@ "):
201+
print(hunk_lines[0])
202+
hunk_lines = hunk_lines[1:] # Operate on the rest of the hunk
203+
header_printed_this_time = True # Flag that header was output
204+
205+
if hunk_lines: # If there are lines left after potentially removing header
206+
lines_to_print_count = args.context_lines
207+
actual_trailing_lines = hunk_lines[-lines_to_print_count:]
208+
for line_content in actual_trailing_lines:
209+
print(line_content)
210+
elif not header_printed_this_time :
211+
pass # Hunk was empty or only a header, already handled by outer 'else' or header print
212+
213+
else: # diff_hunk was None or empty
202214
print("(No diff hunk available for this comment)")
203215
print("```") # End of Markdown code block
204216

@@ -212,16 +224,14 @@ def main():
212224
next_since_dt = latest_created_at_obj.astimezone(timezone.utc) + timedelta(seconds=2)
213225
next_since_str = next_since_dt.strftime('%Y-%m-%dT%H:%M:%SZ')
214226

215-
new_cmd_args = [sys.argv[0]]
216-
skip_next_arg = False
217-
for i in range(1, len(sys.argv)):
218-
if skip_next_arg:
219-
skip_next_arg = False
220-
continue
227+
new_cmd_args = [sys.executable, sys.argv[0]] # Start with interpreter and script path
228+
i = 1 # Start checking from actual arguments in sys.argv
229+
while i < len(sys.argv):
221230
if sys.argv[i] == "--since":
222-
skip_next_arg = True
231+
i += 2 # Skip --since and its value
223232
continue
224233
new_cmd_args.append(sys.argv[i])
234+
i += 1
225235

226236
new_cmd_args.extend(["--since", next_since_str])
227237
suggested_cmd = " ".join(new_cmd_args)

0 commit comments

Comments
 (0)