-
Notifications
You must be signed in to change notification settings - Fork 0
Austinamoruso/filtercomments #35
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
Conversation
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.
Summary of Changes
Hello @austinamorusoyardstick, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the pull request review interface by introducing advanced comment and review filtering capabilities. It allows users to precisely control which types of comments (e.g., general comments, reviews, line comments) are displayed, improving the focus and efficiency of the review process.
Highlights
- Enhanced Filtering Options: Added new boolean fields (
showComments,showReviews,showLineComments,showOnlyLineComments) toPRReviewModelto provide granular control over comment visibility, initialized with sensible defaults. - New Keyboard Shortcuts: Introduced dedicated keyboard shortcuts (
c,r,l,C,R,L) within theUpdatemethod, enabling users to quickly toggle various comment and review filters, including showing only line comments or specific comment types. - Refined Comment Filtering Logic: The
getActiveCommentsmethod has been re-engineered to dynamically filter comments based on their type (review, review_comment, issue_comment) and whether they are line-specific, respecting the new filter settings. - Improved User Interface Feedback: The
ViewandsimpleViewmethods now display a comprehensive status of the active filters, and their help texts have been updated to include the new keyboard shortcuts, ensuring users are aware of the new functionalities.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces some excellent enhancements for filtering comments in the PR review UI. The new filtering options are a great addition for managing large reviews. The implementation is solid, but I've pointed out a few areas for improvement, including some code duplication in the Update method and a logic issue in the view rendering that results in dead code. Addressing these points will make the code more robust and easier to maintain. Overall, great work!
ui/pr_review.go
Outdated
| if m.showOnlyLineComments { | ||
| filterParts = append(filterParts, "showing only line comments") | ||
| } else if !m.showComments && !m.showReviews { | ||
| filterParts = append(filterParts, "hiding all") | ||
| } else if !m.showComments && m.showReviews { | ||
| filterParts = append(filterParts, "showing only reviews") | ||
| } else if m.showComments && !m.showReviews { | ||
| filterParts = append(filterParts, "showing only comments") | ||
| } else if !m.showComments { | ||
| filterParts = append(filterParts, "hiding comments") | ||
| } else if !m.showReviews { | ||
| filterParts = append(filterParts, "hiding reviews") | ||
| } |
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.
There appears to be some dead code in the logic for constructing the filter status string. The conditions else if !m.showComments and else if !m.showReviews are unreachable.
For example, for the else if !m.showComments branch to be reached, !m.showComments must be true. However, the preceding if/else if chain covers all possibilities for m.showReviews when !m.showComments is true:
- If
m.showReviewsis false,!m.showComments && !m.showReviewsis true, and the "hiding all" branch is taken. - If
m.showReviewsis true,!m.showComments && m.showReviewsis true, and the "showing only reviews" branch is taken.
Therefore, the later else if !m.showComments condition can never be met. The same applies to else if !m.showReviews.
I suggest removing the unreachable branches to fix the logic.
if m.showOnlyLineComments {
filterParts = append(filterParts, "showing only line comments")
} else if !m.showComments && !m.showReviews {
filterParts = append(filterParts, "hiding all")
} else if !m.showComments && m.showReviews {
filterParts = append(filterParts, "showing only reviews")
} else if m.showComments && !m.showReviews {
filterParts = append(filterParts, "showing only comments")
}
ui/pr_review.go
Outdated
| if m.showOnlyLineComments { | ||
| filterParts = append(filterParts, "showing only line comments") | ||
| } else if !m.showComments && !m.showReviews { | ||
| filterParts = append(filterParts, "hiding all") | ||
| } else if !m.showComments && m.showReviews { | ||
| filterParts = append(filterParts, "showing only reviews") | ||
| } else if m.showComments && !m.showReviews { | ||
| filterParts = append(filterParts, "showing only comments") | ||
| } else if !m.showComments { | ||
| filterParts = append(filterParts, "hiding comments") | ||
| } else if !m.showReviews { | ||
| filterParts = append(filterParts, "hiding reviews") | ||
| } |
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.
This block has the same dead code issue as the one in the View() method. The else if !m.showComments and else if !m.showReviews branches are unreachable. Please see my other comment for a detailed explanation.
Applying the same fix here will resolve the issue.
if m.showOnlyLineComments {
filterParts = append(filterParts, "showing only line comments")
} else if !m.showComments && !m.showReviews {
filterParts = append(filterParts, "hiding all")
} else if !m.showComments && m.showReviews {
filterParts = append(filterParts, "showing only reviews")
} else if m.showComments && !m.showReviews {
filterParts = append(filterParts, "showing only comments")
}| case "c": | ||
| m.showComments = !m.showComments | ||
| m.showOnlyLineComments = false | ||
| // Reset current index to ensure it's valid | ||
| m.currentIndex = 0 | ||
| if m.ready { | ||
| m.updateViewportContent() | ||
| m.viewport.SetYOffset(0) // Reset scroll position | ||
| } | ||
| return m, nil | ||
|
|
||
| case "r": | ||
| m.showReviews = !m.showReviews | ||
| m.showOnlyLineComments = false | ||
| // Reset current index to ensure it's valid | ||
| m.currentIndex = 0 | ||
| if m.ready { | ||
| m.updateViewportContent() | ||
| m.viewport.SetYOffset(0) // Reset scroll position | ||
| } | ||
| return m, nil | ||
|
|
||
| case "l": | ||
| m.showLineComments = !m.showLineComments | ||
| m.showOnlyLineComments = false | ||
| // Reset current index to ensure it's valid | ||
| m.currentIndex = 0 | ||
| if m.ready { | ||
| m.updateViewportContent() | ||
| m.viewport.SetYOffset(0) // Reset scroll position | ||
| } | ||
| return m, nil | ||
|
|
||
| case "L": | ||
| // Show only line comments | ||
| m.showComments = true | ||
| m.showReviews = true | ||
| m.showLineComments = true | ||
| m.showOnlyLineComments = true | ||
| // Reset current index to ensure it's valid | ||
| m.currentIndex = 0 | ||
| if m.ready { | ||
| m.updateViewportContent() | ||
| m.viewport.SetYOffset(0) // Reset scroll position | ||
| } | ||
| return m, nil | ||
|
|
||
| case "R": | ||
| // Show only reviews | ||
| m.showComments = false | ||
| m.showReviews = true | ||
| m.showLineComments = true | ||
| m.showOnlyLineComments = false | ||
| // Reset current index to ensure it's valid | ||
| m.currentIndex = 0 | ||
| if m.ready { | ||
| m.updateViewportContent() | ||
| m.viewport.SetYOffset(0) // Reset scroll position | ||
| } | ||
| return m, nil | ||
|
|
||
| case "C": | ||
| // Show only comments (not reviews) | ||
| m.showComments = true | ||
| m.showReviews = false | ||
| m.showLineComments = true | ||
| m.showOnlyLineComments = false | ||
| // Reset current index to ensure it's valid | ||
| m.currentIndex = 0 | ||
| if m.ready { | ||
| m.updateViewportContent() | ||
| m.viewport.SetYOffset(0) // Reset scroll position | ||
| } | ||
| return m, nil |
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.
There's significant code duplication across the new filter-handling cases (c, r, l, L, R, C). The block for resetting the view is repeated in each one. This can be refactored into a helper function to improve maintainability and reduce redundancy.
Given that Update is a value-receiver method, you could introduce a helper method to encapsulate this logic. For example:
func (m *PRReviewModel) resetViewAfterFilterChange() {
m.currentIndex = 0
if m.ready {
m.updateViewportContent()
m.viewport.SetYOffset(0) // Reset scroll position
}
}You could then call it from each case like this:
case "c":
m.showComments = !m.showComments
m.showOnlyLineComments = false
(&m).resetViewAfterFilterChange()
return m, nilWhile this works, a more idiomatic approach in bubbletea might be to make Update a pointer-receiver method if it frequently modifies state, which would simplify calls to other pointer-receiver helpers. However, just extracting the duplicated block into a helper is a great improvement.
ui/pr_review.go
Outdated
| var filtered []*git.PRComment | ||
| for _, comment := range comments { | ||
| // Check type filters | ||
| includeByType := true | ||
| switch comment.Type { | ||
| case "review": | ||
| includeByType = m.showReviews | ||
| case "review_comment", "issue_comment": | ||
| includeByType = m.showComments | ||
| } | ||
|
|
||
| // Check line number filter | ||
| includeByLine := true | ||
| if m.showOnlyLineComments { | ||
| // When showing only line comments, include only those with line numbers | ||
| includeByLine = comment.Line > 0 | ||
| } else { | ||
| // Normal line filter behavior | ||
| includeByLine = m.showLineComments || comment.Line == 0 | ||
| } | ||
|
|
||
| // Include comment if it passes all filters | ||
| if includeByType && includeByLine { | ||
| filtered = append(filtered, comment) | ||
| } | ||
| } |
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.
The filtering logic here is correct, but its implementation could be more idiomatic and arguably more readable by using guard clauses with continue to skip comments that don't match the filters. This flattens the logic and avoids nesting if statements.
var filtered []*git.PRComment
for _, comment := range comments {
// Filter by type
switch comment.Type {
case "review":
if !m.showReviews {
continue
}
case "review_comment", "issue_comment":
if !m.showComments {
continue
}
}
// Filter by line number
if m.showOnlyLineComments {
if comment.Line <= 0 {
continue
}
} else if !m.showLineComments && comment.Line > 0 {
continue
}
filtered = append(filtered, comment)
}|
/gemini review |
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.
Code Review
This pull request introduces significant enhancements to the pull request review UI by adding granular filtering options for comments and reviews. The changes are well-structured, with new state fields in the model, corresponding logic in the Update function to handle new keybindings, and updates to the view to reflect filter statuses. The introduction of resetViewAfterFilterChange is a good step towards reducing code duplication.
My review focuses on further improving maintainability and efficiency. I've identified a clear case of duplicated code between the View and simpleView methods that could be refactored into a helper function. Additionally, there's a small performance optimization possible in getActiveComments by pre-allocating the slice for filtered results. Overall, these are solid improvements to the tool's usability.
ui/pr_review.go
Outdated
| } | ||
|
|
||
| // Apply filters | ||
| var filtered []*git.PRComment |
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.
To improve performance and avoid multiple slice reallocations when filtering comments, it's a good practice to pre-allocate the filtered slice with a capacity that matches the number of comments to be filtered. This avoids growing the slice dynamically in the loop.
| var filtered []*git.PRComment | |
| filtered := make([]*git.PRComment, 0, len(comments)) |
ui/pr_review.go
Outdated
| var filterParts []string | ||
| if m.filterEnabled { | ||
| header.WriteString(filterStyle.Render("(Filter: ON - hiding outdated/resolved/gemini)")) | ||
| filterParts = append(filterParts, "Filter: ON") | ||
| } else { | ||
| header.WriteString(filterStyle.Render("(Filter: OFF - showing all comments)")) | ||
| filterParts = append(filterParts, "Filter: OFF") | ||
| } | ||
|
|
||
| // Show comment/review filter status | ||
| if m.showOnlyLineComments { | ||
| filterParts = append(filterParts, "showing only line comments") | ||
| } else if !m.showComments && !m.showReviews { | ||
| filterParts = append(filterParts, "hiding all") | ||
| } else if !m.showComments && m.showReviews { | ||
| filterParts = append(filterParts, "showing only reviews") | ||
| } else if m.showComments && !m.showReviews { | ||
| filterParts = append(filterParts, "showing only comments") | ||
| } | ||
|
|
||
| // Show line comments filter status (only if not in "show only line comments" mode) | ||
| if !m.showOnlyLineComments && !m.showLineComments { | ||
| filterParts = append(filterParts, "hiding line comments") | ||
| } | ||
|
|
||
| filterStatus := "(" + strings.Join(filterParts, " - ") | ||
| if m.filterEnabled { | ||
| filterStatus += " - hiding outdated/resolved/gemini" | ||
| } | ||
| filterStatus += ")" | ||
|
|
||
| header.WriteString(filterStyle.Render(filterStatus)) |
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.
This block of code for building the filter status string is duplicated in the simpleView function (lines 711-740). To improve maintainability and reduce redundancy, consider extracting this logic into a new helper method on PRReviewModel.
For example, you could add the following method:
func (m PRReviewModel) buildFilterStatus() string {
var filterParts []string
if m.filterEnabled {
filterParts = append(filterParts, "Filter: ON")
} else {
filterParts = append(filterParts, "Filter: OFF")
}
// Show comment/review filter status
if m.showOnlyLineComments {
filterParts = append(filterParts, "showing only line comments")
} else if !m.showComments && !m.showReviews {
filterParts = append(filterParts, "hiding all")
} else if !m.showComments && m.showReviews {
filterParts = append(filterParts, "showing only reviews")
} else if m.showComments && !m.showReviews {
filterParts = append(filterParts, "showing only comments")
}
// Show line comments filter status (only if not in \"show only line comments\" mode)
if !m.showOnlyLineComments && !m.showLineComments {
filterParts = append(filterParts, "hiding line comments")
}
filterStatus := "(" + strings.Join(filterParts, " - ")
if m.filterEnabled {
filterStatus += " - hiding outdated/resolved/gemini"
}
filterStatus += ")"
return filterStatus
}Then you can replace this entire block with a single call to this new method.
| var filterParts []string | |
| if m.filterEnabled { | |
| header.WriteString(filterStyle.Render("(Filter: ON - hiding outdated/resolved/gemini)")) | |
| filterParts = append(filterParts, "Filter: ON") | |
| } else { | |
| header.WriteString(filterStyle.Render("(Filter: OFF - showing all comments)")) | |
| filterParts = append(filterParts, "Filter: OFF") | |
| } | |
| // Show comment/review filter status | |
| if m.showOnlyLineComments { | |
| filterParts = append(filterParts, "showing only line comments") | |
| } else if !m.showComments && !m.showReviews { | |
| filterParts = append(filterParts, "hiding all") | |
| } else if !m.showComments && m.showReviews { | |
| filterParts = append(filterParts, "showing only reviews") | |
| } else if m.showComments && !m.showReviews { | |
| filterParts = append(filterParts, "showing only comments") | |
| } | |
| // Show line comments filter status (only if not in "show only line comments" mode) | |
| if !m.showOnlyLineComments && !m.showLineComments { | |
| filterParts = append(filterParts, "hiding line comments") | |
| } | |
| filterStatus := "(" + strings.Join(filterParts, " - ") | |
| if m.filterEnabled { | |
| filterStatus += " - hiding outdated/resolved/gemini" | |
| } | |
| filterStatus += ")" | |
| header.WriteString(filterStyle.Render(filterStatus)) | |
| header.WriteString(filterStyle.Render(m.buildFilterStatus())) |
|
/gemini review |
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.
Code Review
This pull request introduces a valuable set of filtering features for pull request comments, enhancing user control over the displayed information. The implementation is well-structured, with new logic for handling filter states and updating the UI accordingly.
My review focuses on a few areas to improve the user experience and maintainability:
- Refining the filter status message to avoid redundancy.
- Adjusting the behavior of filter shortcuts to be more intuitive and respect user settings.
- Adding the new keybindings to the simple view.
Overall, these are great additions that make the tool more powerful. The suggested changes are minor and aim to polish the new functionality.
| m.showComments = false | ||
| m.showReviews = true | ||
| m.showLineComments = true | ||
| m.showOnlyLineComments = false | ||
| m = m.resetViewAfterFilterChange() | ||
| return m, nil |
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.
The 'R' shortcut (show only reviews) forces showLineComments to true, which overrides the user's preference if they have previously toggled line comments off using 'l'. This behavior can be unintuitive.
To provide a more predictable experience, this shortcut should respect the existing showLineComments setting rather than resetting it. The same principle applies to the 'C' shortcut.
| m.showComments = false | |
| m.showReviews = true | |
| m.showLineComments = true | |
| m.showOnlyLineComments = false | |
| m = m.resetViewAfterFilterChange() | |
| return m, nil | |
| m.showReviews = true | |
| m.showOnlyLineComments = false |
| case "C": | ||
| // Show only comments (not reviews) | ||
| m.showComments = true | ||
| m.showReviews = false | ||
| m.showLineComments = true | ||
| m.showOnlyLineComments = false | ||
| m = m.resetViewAfterFilterChange() |
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.
Similar to the 'R' shortcut, the 'C' shortcut (show only comments) forces showLineComments to true. This overrides the user's preference set with the 'l' key, which can be unexpected.
This shortcut should respect the existing showLineComments setting to ensure consistent and predictable filter behavior.
| case "C": | |
| // Show only comments (not reviews) | |
| m.showComments = true | |
| m.showReviews = false | |
| m.showLineComments = true | |
| m.showOnlyLineComments = false | |
| m = m.resetViewAfterFilterChange() | |
| m.showComments = true | |
| m.showReviews = false | |
| m.showOnlyLineComments = false |
ui/pr_review.go
Outdated
| // Show line comments filter status (only if not in "show only line comments" mode) | ||
| if !m.showOnlyLineComments && !m.showLineComments { | ||
| filterParts = append(filterParts, "hiding line comments") |
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.
The current logic for displaying the "hiding line comments" status can be confusing. For example, if a user chooses to show only reviews (which don't have line numbers), the status might also include 'hiding line comments', which is redundant and potentially misleading.
To improve clarity, the "hiding line comments" status should only be shown when it's relevant—that is, when comments that can be associated with specific lines are actually being displayed.
if !m.showOnlyLineComments && m.showComments && m.showReviews && !m.showLineComments {
ui/pr_review.go
Outdated
| "c:toggle comments", | ||
| "r:toggle reviews", | ||
| "l:toggle line comments", |
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.
|
/gemini review |
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.
Code Review
This pull request introduces valuable new filtering capabilities for pull request comments. The implementation is well-structured, and the new features are a significant improvement. I have a couple of suggestions to enhance the clarity of the filter status and refine the state management for one of the new filter options.
| m.showComments = true | ||
| m.showReviews = true | ||
| m.showLineComments = true | ||
| m.showOnlyLineComments = true |
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.
In the handler for the 'L' key, m.showReviews is set to true. This appears to be redundant. When 'show only line comments' is active, it filters for comments where Line > 0. Since review type comments (controlled by showReviews) don't have line numbers, they are filtered out regardless of the showReviews flag.
Setting m.showReviews = true here could be confusing if a user has intentionally disabled reviews, as it changes the filter state without affecting the immediate output. To make the state transition more predictable, I recommend removing this line.
| m.showComments = true | |
| m.showReviews = true | |
| m.showLineComments = true | |
| m.showOnlyLineComments = true | |
| m.showComments = true | |
| m.showLineComments = true | |
| m.showOnlyLineComments = true |
| filterStatus := "(" + strings.Join(filterParts, " - ") | ||
| if m.filterEnabled { | ||
| filterStatus += " - hiding outdated/resolved/gemini" | ||
| } | ||
| filterStatus += ")" |
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.
The refactoring to use buildFilterStatus is a great improvement for code clarity. However, there's a small regression in the UI's informativeness. Previously, when filterEnabled was false, the UI displayed (Filter: OFF - showing all comments). With the new implementation, it just shows (Filter: OFF) when no other filters are active.
To restore the previous level of clarity for the user, I suggest adding back the "showing all comments" text when the main filter is off and no other sub-filters are active.
filterStatus := "(" + strings.Join(filterParts, " - ")
if m.filterEnabled {
filterStatus += " - hiding outdated/resolved/gemini"
} else if len(filterParts) == 1 {
// When filter is off and no other filters are active, clarify that all comments are shown.
filterStatus += " - showing all comments"
}
filterStatus += ")"| "c/C:toggle/only comments", | ||
| "r/R:toggle/only reviews", | ||
| "l/L:toggle/only line comments", |
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.
These new keybindings are a great addition to the UI, but it would be helpful to indicate that the 'c', 'r', and 'l' keys toggle the filter, while 'C', 'R', and 'L' show only the selected type. Consider rephrasing the help text to clarify this distinction.
"c:toggle comments",
"C:only comments",
"r:toggle reviews",
"R:only reviews",
"l:toggle line comments",
"L:only line comments",… on the length of `comments`, which is a performance optimization to avoid repeated slice reallocations during the filtering loop. Preallocate filtered slice with initial capacity for better performance
3ab4671 to
068b6ec
Compare
|
/gemini review |
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.
Code Review
This pull request introduces new filtering options for PR comments, enhancing the user experience. The changes include new boolean fields in PRReviewModel, updates to the Update method for handling keyboard shortcuts, and modifications to the View and simpleView methods to display the filter status. The addition of the resetViewAfterFilterChange and buildFilterStatus methods improves code organization and readability. Consider defining constants for comment types and keybindings to improve maintainability.
| m = m.resetViewAfterFilterChange() | ||
| return m, nil |
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.
| m.showComments = !m.showComments | ||
| m.showOnlyLineComments = false |
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.
| switch comment.Type { | ||
| case "review": | ||
| if !m.showReviews { | ||
| continue | ||
| } | ||
| case "review_comment", "issue_comment": | ||
| if !m.showComments { | ||
| continue | ||
| } |
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.
| "c/C:toggle/only comments", | ||
| "r/R:toggle/only reviews", | ||
| "l/L:toggle/only line comments", |
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.
|
|
||
| b.WriteString("\n") | ||
| b.WriteString(helpStyle.Render("Keys: j/k:nav • a/d:accept/deny • e:expand • s:split • f:toggle filter • Enter:process • q:cancel")) | ||
| b.WriteString(helpStyle.Render("Keys: j/k:nav • a/d:accept/deny • e:expand • s:split • f:toggle filter • c/C:toggle/only comments • r/R:toggle/only reviews • l/L:toggle/only line comments • Enter:process • q:cancel")) |
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.
This pull request introduces enhancements to the
PRReviewModelinui/pr_review.goto provide more granular control over filtering and viewing pull request comments and reviews. The key changes include the addition of new filtering options, updates to the user interface to reflect these options, and improved logic for filtering comments based on user preferences.New Filtering Features
showComments,showReviews,showLineComments,showOnlyLineComments) toPRReviewModelto track filtering states.NewPRReviewModelconstructor to initialize these fields with default values.Enhanced Filtering Logic
Updatemethod to handle new keyboard shortcuts (c,r,l,C,R,L) for toggling filters and resetting the view accordingly.getActiveCommentsmethod to filter comments dynamically based on the new filtering options (e.g., type of comment, line number).User Interface Improvements
ViewandsimpleViewmethods to display the current filter status in a human-readable format, reflecting the new filtering options. [1] [2]ViewandsimpleViewto include the new keyboard shortcuts for toggling filters. [1] [2]