Skip to content

Conversation

ImmortalRabbit
Copy link
Contributor

It seems getSkipPoint suppose to return either number or undefined but it's actually never returns number

@ImmortalRabbit
Copy link
Contributor Author

It looks like npm install timed out because of network issue, is there anyway to re-trigger it?

@lukasmasuch lukasmasuch requested a review from Copilot August 14, 2025 12:06
Copy link
Contributor

@Copilot Copilot AI left a comment

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 fixes a bug in the getSkipPoint function where it was missing a return statement, causing it to always return undefined instead of the intended number value. The fix adds the missing return statement to return the calculated drawRegionsLowestY value.

Key Changes

  • Adds missing return statement to getSkipPoint function

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -7,6 +7,7 @@ export function getSkipPoint(drawRegions: readonly Rectangle[]): number | undefi
for (const dr of drawRegions) {
drawRegionsLowestY = Math.min(drawRegionsLowestY ?? dr.y, dr.y);
}
return drawRegionsLowestY;
Copy link
Preview

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

The function returns drawRegionsLowestY which is initialized as undefined and will remain undefined if the drawRegions array is empty. Consider returning a default value or handling the empty array case explicitly to match the function's return type contract.

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

@lukasmasuch lukasmasuch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lukasmasuch lukasmasuch merged commit 09d3f65 into glideapps:main Aug 14, 2025
3 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