Conversation
Summary of ChangesHello @waruqi, 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 refines the package download process by implementing a more robust method for identifying file extensions. A new utility function, Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a fix to correctly handle package extensions for non-archive files, such as .exe or .sh. This is achieved by adding a new helper function, _url_extension, and using it in place of archive.extension where appropriate. The change is logical and correctly addresses the issue. However, I've identified a potential issue in the new helper function where URLs with query parameters are not handled correctly, which could lead to incorrect extension parsing. I have provided a suggestion to make the function more robust.
| function _url_extension(url) | ||
| local extension = archive.extension(url) | ||
| if extension == "" then | ||
| -- maybe non-archive file, e.g. .exe, .sh, .. | ||
| extension = path.extension(url) | ||
| end | ||
| return extension | ||
| end |
There was a problem hiding this comment.
The new _url_extension function does not correctly handle URLs that contain query parameters (e.g., https://example.com/file.zip?token=...). This can result in an incorrect extension like .zip?token=... being extracted. To ensure robustness, the query string should be stripped from the URL before attempting to determine the file extension.
function _url_extension(url)
local urlpath = url:split('?', {plain = true})[1]
local extension = archive.extension(urlpath)
if extension == "" then
-- maybe non-archive file, e.g. .exe, .sh, ..
extension = path.extension(urlpath)
end
return extension
end
xmake-io/xmake-repo#8136