- Notifications
You must be signed in to change notification settings - Fork1.4k
Enhance get_pull_request_diff tool with pagination and file filtering options#627
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
… optionsSigned-off-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Enhance theget_pull_request_diff tool by adding pagination and file-filtering capabilities to handle large pull requests efficiently.
- Introduces new parameters:
fileList,files,pathPrefix,page, andperPage - Implements paginated list mode and specific-file/path-prefix diff modes
- Updates tests, toolsnap, and README to cover new functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/github/pullrequests.go | Added imports, parameters, and logic for pagination and filters |
| pkg/github/pullrequests_test.go | Added tests for file list mode with pagination, path prefix, specific files, and full diff |
| pkg/github/toolsnaps/get_pull_request_diff.snap | Updated snapshot to include new input schema properties |
| README.md | Documented new parameters (fileList,files,pathPrefix,page,perPage) |
Comments suppressed due to low confidence (4)
pkg/github/pullrequests.go:1689
- The branch handling
pathPrefixwithoutfileListisn’t covered by tests; add a test case invokingpathPrefixalone to verify correct diff output.
if len(params.Files) > 0 || params.PathPrefix != "" {pkg/github/pullrequests.go:9
- The code uses json.Marshal in the fileList block but the import for "encoding/json" is missing; add
import "encoding/json".
"strings"pkg/github/pullrequests.go:1694
- Each call to ListFiles returns a Response with an open body; add
defer resp.Body.Close()inside the loop to avoid leaking HTTP connections.
files, resp, err := client.PullRequests.ListFiles(ctx, params.Owner, params.Repo, int(params.PullNumber), opts)pkg/github/pullrequests.go:1660
- [nitpick] Filtering by
PathPrefixis duplicated in both the fileList and specific-files branches; consider extracting a reusable helper function.
if params.PathPrefix != "" {
Uh oh!
There was an error while loading.Please reload this page.
Signed-off-by: Tiger Kaovilaipassawit.kaovilai@gmail.com
Closes:#625