Skip to content

Conversation

speige
Copy link
Contributor

@speige speige commented Apr 14, 2025

&first= query param is not the page count but the image count, so sending the pageCount is only incrementing by 1 image and getting identical results to the last query except 1 new image.

html == "" is not a reliable way to check for end of results because bing will just keep returning the last page of images if there are no more images available

@KTS-o7
Copy link
Owner

KTS-o7 commented Apr 14, 2025

Thank you !
Will be reviewing and merging within 1-2 days !

@KTS-o7 KTS-o7 requested a review from Copilot April 15, 2025 14:35
Copy link

@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 paging and end-of-results detection when downloading images from Bing. The changes update the paging parameters by using a hard-coded page size multiplier and add a check to break out of the loop when no new images are found.

  • Update paging calculation to multiply the page counter by a hard-coded page size.
  • Replace the previous '&first' and '&count' parameters with updated values based on the page size.
  • Introduce a flag to detect when no new images are found and break out of the loop.

found_image = True

if not found_image:
logging.info("[%] No more images are available")
Copy link
Preview

Copilot AI Apr 15, 2025

Choose a reason for hiding this comment

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

The log message uses '[%]' which appears to be a formatting placeholder that is not being replaced; consider updating it to include meaningful context or removing the placeholder.

Suggested change
logging.info("[%] No more images are available")
logging.info("No more images are available")

Copilot uses AI. Check for mistakes.

@KTS-o7 KTS-o7 mentioned this pull request Apr 15, 2025
@KTS-o7 KTS-o7 closed this in #19 Apr 15, 2025
@KTS-o7 KTS-o7 merged commit 785281c into KTS-o7:main Apr 15, 2025
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