Skip to content

fix(views:about): default text for adb version #918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 29, 2025

Conversation

Rudxain
Copy link
Member

@Rudxain Rudxain commented Mar 27, 2025

Rudxain added 2 commits March 27, 2025 18:41
Change the method signature to
allow the consumer to decide when/if and how to parse the `stdout`
@Rudxain Rudxain requested a review from Copilot March 27, 2025 23:01
@Rudxain Rudxain self-assigned this Mar 27, 2025
@Rudxain Rudxain linked an issue Mar 27, 2025 that may be closed by this pull request
3 tasks
@Rudxain Rudxain requested a review from a team March 27, 2025 23:02
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 regression in the ADB version display on the About page while also improving performance by simplifying how the ADB version is parsed and returned.

  • Simplifies the ADB version extraction in the GUI view
  • Changes the ADB version function to return a String instead of a vector of lines
  • Adds debug assertions in the ADB command for output validation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/gui/views/about.rs Updates version text extraction and error handling
src/core/adb.rs Changes the version function return type and refines assertions
Comments suppressed due to low confidence (1)

src/gui/views/about.rs:102

  • Consider handling the case when the version string does not contain any newline more gracefully instead of using unreachable!(), which could result in a panic in release builds.
.nth(0)
                .unwrap_or_else(|| unreachable!())

@Rudxain
Copy link
Member Author

Rudxain commented Mar 27, 2025

Copilot is wrong about the assertions. I just refactored them into their own for loop

@Rudxain
Copy link
Member Author

Rudxain commented Mar 28, 2025

Forgot the screenshots:
About showing ADB version
About showing "Couldn't fetch ADB version. Is it installed?"

@AnonymousWP AnonymousWP added the bug Something isn't working label Mar 28, 2025
@adhirajsinghchauhan adhirajsinghchauhan merged commit 1890a21 into main Apr 29, 2025
18 checks passed
@adhirajsinghchauhan adhirajsinghchauhan deleted the bug/about/panic-adb-version#917 branch April 29, 2025 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(about): panic when displaying ADB version
3 participants