Skip to content

fix: avoid duplicating the last page#19

Merged
rueian merged 2 commits into
mainfrom
avoid-dup-last-page
May 3, 2025
Merged

fix: avoid duplicating the last page#19
rueian merged 2 commits into
mainfrom
avoid-dup-last-page

Conversation

@rueian

@rueian rueian commented May 3, 2025

Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Rueian <rueiancsie@gmail.com>
@rueian rueian requested a review from Copilot May 3, 2025 18:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR aims to fix the duplicate handling of the last page in file retrieval logic.

  • Updated the condition in getPage to include an additional check for the duplicate last page scenario.

Comment thread file.go
return nil, 0, 0, err
}
if pageEnd <= f.tc {
if pageEnd <= f.tc || (pageOff < f.tc && f.sz == f.tc && f.ptr.Size == f.sz) {

Copilot AI May 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The compound conditional is complex and may hinder future maintenance. Consider refactoring by extracting the condition into well-named boolean variables or adding an inline comment to clarify the logic.

Suggested change
if pageEnd <= f.tc || (pageOff < f.tc && f.sz == f.tc && f.ptr.Size == f.sz) {
isPageEndWithinThreshold := pageEnd <= f.tc
isPageOffWithinThresholdAndSizesMatch := pageOff < f.tc && f.sz == f.tc && f.ptr.Size == f.sz
if isPageEndWithinThreshold || isPageOffWithinThresholdAndSizesMatch {

Copilot uses AI. Check for mistakes.
@codecov

codecov Bot commented May 3, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

Signed-off-by: Rueian <rueiancsie@gmail.com>
@rueian rueian changed the title fix: avoid duplicate the last page fix: avoid duplicating the last page May 3, 2025
@rueian rueian merged commit f846b29 into main May 3, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants