limit the number of parts before creating a multi-part zip#63
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @loewy78, 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 introduces a crucial user experience improvement by implementing client-side validation for multi-part ZIP creation. It prevents users from generating archives with an unmanageably high number of parts, which could lead to poor performance or usability issues, especially on mobile devices. The validation provides immediate feedback to the user on the input field, guiding them to choose appropriate split sizes.
Highlights
- Multi-part ZIP Validation: Implemented client-side validation to limit the number of parts generated for multi-part ZIP archives. This prevents the creation of excessively fragmented archives that could degrade user experience, especially on mobile devices.
- User Feedback: An error message is now displayed directly on the input field (
splitSizeInput) if the configured split size would result in more than 100 parts, providing immediate and clear feedback to the user. - Core Logic & Utility: A new utility function,
getMultiZipPartsCount, was added to calculate the total number of parts a split ZIP would generate, taking into account the total size of selected files and the desired split size. This calculation is performed asynchronously to avoid blocking the UI.
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.
Code Review
This pull request introduces a limit on the number of parts for a multi-part zip archive to improve user experience. The implementation correctly adds asynchronous validation before starting the archiving process. The changes are well-contained and the logic is sound. I've provided a couple of suggestions to improve code conciseness and maintainability.
| val division = selectedFilesSize / splitZipSize | ||
| val remainder = selectedFilesSize % splitZipSize | ||
| return if (remainder > 0) division + 1 else division |
There was a problem hiding this comment.
This function can be made more concise and efficient by using a common integer arithmetic pattern for ceiling division. This avoids the need for intermediate variables and the modulo operator.
if (splitZipSize <= 0) {
return Long.MAX_VALUE
}
return (selectedFilesSize + splitZipSize - 1) / splitZipSize| if (partsCount > MAX_MULTI_ZIP_PARTS) { | ||
| splitSizeInput.error = getString( | ||
| com.wirelessalien.zipxtract.R.string.error_too_many_parts, | ||
| partsCount, | ||
| MAX_MULTI_ZIP_PARTS | ||
| ) | ||
| return@launch | ||
| } else { | ||
| splitSizeInput.error = null | ||
| } |
There was a problem hiding this comment.
The else block here is redundant because the if block contains a return statement. You can simplify this logic by removing the else and placing splitSizeInput.error = null after the if block. This makes the code more linear and easier to read.
if (partsCount > MAX_MULTI_ZIP_PARTS) {
splitSizeInput.error = getString(
com.wirelessalien.zipxtract.R.string.error_too_many_parts,
partsCount,
MAX_MULTI_ZIP_PARTS
)
return@launch
}
splitSizeInput.error = null| mainFragment?.convertToBytes(splitSizeText.toLongOrNull() ?: 64, splitSizeUnit) | ||
| } else { | ||
| null | ||
| val splitZipSize = mainFragment.convertToBytes(splitSizeText, splitSizeUnit) |
There was a problem hiding this comment.
Can I help with a specific point?
|
Thank you |
Added asynchronous validation of the number of parts before launching the split ZIP service, with an error displayed directly on the input field. This check (max. 100 files for example) prevents an incorrect unit or value entered from generating too many files, which can make exploring the folder impossible on a mobile device and degrade the user experience.