Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue can't be closed if title length exceeds 80 characters #244

Open
rgalonso opened this issue Nov 12, 2024 · 5 comments
Open

issue can't be closed if title length exceeds 80 characters #244

rgalonso opened this issue Nov 12, 2024 · 5 comments

Comments

@rgalonso
Copy link
Contributor

When an issue is created and the title is long (> 80 characters), it is truncated and appended with .... When the issue is closed and a search is performed on the title, this truncation isn't accounted for, preventing the issue from being found and closed.

@rgalonso
Copy link
Contributor Author

@alstr, this issue occurs in the client code, so the solution I have locally is just for GitLab. Once I'm ready to release the GitLab version, you can see how I've implemented the solution, in case you want to handle it the same way in GitHubClient. Or just let me know if you want to see the solution here as a comment.

@alstr
Copy link
Owner

alstr commented Nov 13, 2024

Yes, that would be good to see.

From memory if the URL insertion is enabled it should use that method to find an issue first, but then if not use title lookup.

The limit is probably overly considervative; I think the GitHub issue title max length is actually 256 characters, so it could be increased also. GitLab is 255 apparently.

@rgalonso
Copy link
Contributor Author

Here's the diff from close_issue. I imagine you'll understand the context of where it belongs.

+            # if title length is long, make sure we're searching using
+            # the exact same title as would've been inserted
+            search_title = issue.title + '...' if len(issue.title) > self.max_issue_title_length else issue.title
             for existing_issue in self.existing_issues:
-                if existing_issue['title'] == issue.title:
+                if existing_issue['title'] == search_title:
                     matched += 1
                     # If there are multiple issues with similar titles, don't try and close any.
                     if matched > 1:
-                        print(f'Skipping issue (multiple matches)')
+                        print(f'Skipping issue closure due to ambiguous match against multiple existing issues, shown below')
+                        for x in self.existing_issues:
+                            if x['title'] == search_title:
+                                print(f' {x["web_url"]}')
+                        issue_number = None
                         break

So, basically, there's a new variable search_title which is equivalent to issue_title, but truncated if needed. So everywhere you were referencing issue_title, just replace it with search_title.

The other item worth noting here is that I made the "multiple matches" error condition have more information to help the user. Take it or leave it, but if you use it, be sure to use the correct dictionary key to get the URL of the issue. (See second-to-last added line.) For GitLab, that's web_url, but for GitHub, it appears to be simply url.

You'll also notice that it's referencing self.max_issue_title_length rather than a hard-coded 80. I initialize that in __init__ and also reference it in create_issue to ensure that if it gets changed, it's consistent everywhere.

-        title = title + '...' if len(title) > 80 else title
+        title = title + '...' if len(title) > self.max_issue_title_length else title

@rgalonso
Copy link
Contributor Author

One more thing: the last added line resets issue_number to None in the error path.

This is needed because if there are multiple matches, then you necessarily will have already gone through the loop at least one time and have already set issue_number to some value. So what happens is that you end up arbitrarily closing the first of the multiple matches, even though you told the user you're skipping it. Setting it to None ensures no issue is closed in this scenario.

@alstr
Copy link
Owner

alstr commented Nov 22, 2024

Thanks for that, I'll look at updating GitHubClient when I get chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants