From 948c3e0507fad4680696ec3895eec1848a34492a Mon Sep 17 00:00:00 2001 From: Robert Alonso <17463757+rgalonso@users.noreply.github.com> Date: Tue, 12 Nov 2024 03:38:07 +0000 Subject: [PATCH] fix: don't include source with issue URL comment If TODO comment is a suffix to a line of (executable) source, don't repeat the source content when inserting the issue URL. But be sure to still keep the same alignment. Closes #229 --- Issue.py | 3 ++- TodoParser.py | 28 +++++++++++++++++++--------- main.py | 2 +- tests/test_process_diff.py | 4 ---- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/Issue.py b/Issue.py index 36b50f6..9b2c237 100644 --- a/Issue.py +++ b/Issue.py @@ -2,7 +2,7 @@ class Issue(object): """Basic Issue model for collecting the necessary info to send to GitHub.""" def __init__(self, title, labels, assignees, milestone, body, hunk, file_name, - start_line, num_lines, markdown_language, status, identifier, identifier_actual, ref, issue_url, issue_number, start_line_within_hunk=1): + start_line, num_lines, prefix, markdown_language, status, identifier, identifier_actual, ref, issue_url, issue_number, start_line_within_hunk=1): self.title = title self.labels = labels self.assignees = assignees @@ -12,6 +12,7 @@ def __init__(self, title, labels, assignees, milestone, body, hunk, file_name, self.file_name = file_name self.start_line = start_line self.num_lines = num_lines + self.prefix = prefix self.markdown_language = markdown_language self.status = status self.identifier = identifier diff --git a/TodoParser.py b/TodoParser.py index 859be83..422adeb 100644 --- a/TodoParser.py +++ b/TodoParser.py @@ -403,7 +403,7 @@ def _extract_issue_if_exists(self, comment_block, marker, hunk_info): for line_number_within_comment_block, line in enumerate(comment_lines): line_status, committed_line = self._get_line_status(line) line_statuses.append(line_status) - cleaned_line = self._clean_line(committed_line, marker) + cleaned_line, pre_marker_length, post_marker_length = self._clean_line(committed_line, marker) line_title, ref, identifier, identifier_actual = self._get_title(cleaned_line) if line_title: if prev_line_title and line_status == line_statuses[-2]: @@ -423,6 +423,7 @@ def _extract_issue_if_exists(self, comment_block, marker, hunk_info): + comment_block['start'] + line_number_within_comment_block), start_line_within_hunk=comment_block['start'] + line_number_within_comment_block + 1, num_lines=1, + prefix=(' '*pre_marker_length)+(marker['pattern'] if marker['type'] == 'line' else '')+(' '*post_marker_length), markdown_language=hunk_info['markdown_language'], status=line_status, identifier=identifier, @@ -532,8 +533,10 @@ def _get_line_status(self, comment): @staticmethod def _clean_line(comment, marker): """Remove unwanted symbols and whitespace.""" - comment = comment.strip() + post_marker_length = 0 if marker['type'] == 'block': + original_comment = comment + comment = comment.strip() start_pattern = r'^' + marker['pattern']['start'] end_pattern = marker['pattern']['end'] + r'$' comment = re.sub(start_pattern, '', comment) @@ -541,10 +544,17 @@ def _clean_line(comment, marker): # Some block comments might have an asterisk on each line. if '*' in start_pattern and comment.startswith('*'): comment = comment.lstrip('*') + comment = comment.strip() + pre_marker_length = original_comment.find(comment) else: - pattern = r'^' + marker['pattern'] - comment = re.sub(pattern, '', comment) - return comment.strip() + comment_segments = re.search(fr'^(.*?)({marker["pattern"]})(\s*)(.*)', comment) + if comment_segments: + pre_marker_text, _, post_marker_whitespace, comment = comment_segments.groups() + pre_marker_length = len(pre_marker_text) + post_marker_length = len(post_marker_whitespace) + else: + pre_marker_length = 0 + return comment, pre_marker_length, post_marker_length def _get_title(self, comment): """Check the passed comment for a new issue title (and reference, if specified).""" @@ -553,13 +563,13 @@ def _get_title(self, comment): title_identifier_actual = None title_identifier = None for identifier in self.identifiers: - title_pattern = re.compile(fr'(\b{re.escape(identifier)}\b)(\(([^)]+)\))?\s*:?\s*(.+)', re.IGNORECASE) + title_pattern = re.compile(fr'(^.*?)(\s*?)(\b{re.escape(identifier)}\b)(\(([^)]+)\))?\s*:?\s*(.+)', re.IGNORECASE) title_search = title_pattern.search(comment) if title_search: - title_identifier_actual = title_search.group(1) + title_identifier_actual = title_search.group(3) title_identifier = identifier - ref = title_search.group(3) # may be empty, which is OK - title = title_search.group(4) + ref = title_search.group(5) # may be empty, which is OK + title = title_search.group(6) break return title, ref, title_identifier, title_identifier_actual diff --git a/main.py b/main.py index c6d3490..b9d594f 100644 --- a/main.py +++ b/main.py @@ -78,7 +78,7 @@ def process_diff(diff, client=Client(), insert_issue_urls=False, parser=TodoPars old_line = file_lines[line_number] remove = fr'(?i:{raw_issue.identifier}).*{re.escape(raw_issue.title)}' insert = f'Issue URL: {client.get_issue_url(new_issue_number)}' - new_line = re.sub(remove, insert, old_line) + new_line = re.sub('^.*'+remove, raw_issue.prefix + insert, old_line) # make sure the above operation worked as intended if new_line != old_line: # Check if the URL line already exists, if so abort. diff --git a/tests/test_process_diff.py b/tests/test_process_diff.py index f318bb2..32556c2 100644 --- a/tests/test_process_diff.py +++ b/tests/test_process_diff.py @@ -110,10 +110,6 @@ def test_same_title_in_same_file(self): self._setUp(['test_same_title_in_same_file.diff']) self._standardTest(5) - # There is a known bug related to this issue, so until it's resolved - # this is an expected failure. - # See #229 - @unittest.expectedFailure def test_comment_suffix_after_source_line(self): self._setUp(['test_comment_suffix_after_source_line.diff']) self._standardTest(1)