Skip to content

fix(tweaks): unbreak main — suspend getString in coroutine scope#577

Merged
rainxchzed merged 1 commit into
mainfrom
fix/tweaks-suspend-getstring
May 12, 2026
Merged

fix(tweaks): unbreak main — suspend getString in coroutine scope#577
rainxchzed merged 1 commit into
mainfrom
fix/tweaks-suspend-getstring

Conversation

@rainxchzed
Copy link
Copy Markdown
Member

@rainxchzed rainxchzed commented May 11, 2026

Main is broken after #576 merged. The proxy-host validation path calls getString(Res.string.proxy_host_*) outside a coroutine — getString is a Compose Resources suspend function. Compiler error:

TweaksViewModel.kt:641:49 Suspend function 'suspend fun getString(resource: StringResource): String' can only be called from a coroutine or another suspend function.

(Also at lines 643, 1027, 1029.)

Fix: move the getString calls inside the existing viewModelScope.launch { … } block; pass the isBlank boolean across the boundary. Same fix in both OnProxySave and buildProxyConfigForTest. Verified locally with ./gradlew :feature:tweaks:presentation:compileDebugKotlinAndroid :feature:tweaks:presentation:compileKotlinJvm — both green. Apologies for shipping the suspend bug into #576 without a compile pass.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Enhanced proxy host validation error messages to better distinguish between empty and invalid host inputs when saving or testing proxy configuration.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1d3ea97e-d9cf-4adc-a7ac-fd83c594c7e9

📥 Commits

Reviewing files that changed from the base of the PR and between 3ffa72f and 04b7212.

📒 Files selected for processing (1)
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt

Walkthrough

TweaksViewModel refactors proxy host validation across two flows: when saving proxy settings and when testing proxy configuration. Both now consistently compute an isBlank flag and select between localized error messages accordingly.

Changes

Proxy Host Validation Message Selection

Layer / File(s) Summary
Validation Message Refactor
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
In OnProxySave and buildProxyConfigForTest flows, compute isBlank flag from form.host.isBlank() and use it to select between proxy_host_required and proxy_host_invalid error messages before emitting the error event. Added import for proxy_host_invalid resource.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A proxy that's blank? We cry "required!"
A proxy with holes? Still mis-fired.
Two validation spots, one simple truth—
The host field must shine in its youth. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly describes the main fix: moving suspend function calls into a coroutine scope in the TweaksViewModel to resolve a compiler error that broke the main branch.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/tweaks-suspend-getstring

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 11, 2026

Greptile Summary

  • Fixes compilation errors from fix(tweaks): proxy host validation + save-needs-two-taps #576 by moving getString (a Compose Resources suspend function) inside viewModelScope.launch { } blocks in both OnProxySave and buildProxyConfigForTest. The isBlank boolean is correctly captured before crossing the coroutine boundary.
  • Also adds the previously-missing import for proxy_host_invalid, which was causing a compile error independently of the suspend-context issue.
  • The fix matches the pre-existing pattern used for invalid_proxy_port validation at lines 626–634, keeping the codebase consistent.

Confidence Score: 5/5

Safe to merge — targeted compilation fix with no logic regressions.

The change is minimal and surgical: it only moves two existing expression groups inside viewModelScope.launch { } scopes and adds one missing import. The isBlank snapshot is correctly taken before the coroutine boundary. The pattern mirrors the already-working port-error path. No new logic is introduced and no P1/P0 issues were found.

No files require special attention.

Important Files Changed

Filename Overview
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt Moves suspend getString calls inside viewModelScope.launch { } in two host-validation paths and adds a missing import; fix is logically correct and consistent with existing port-error pattern.

Sequence Diagram

sequenceDiagram
    participant UI
    participant onAction
    participant viewModelScope
    participant events as _events

    UI->>onAction: OnProxySave / OnProxyTest
    onAction->>onAction: isValidProxyHost returns false
    onAction->>onAction: "capture isBlank = form.host.isBlank()"
    onAction->>viewModelScope: launch coroutine
    activate viewModelScope
    onAction-->>onAction: return (non-local)
    viewModelScope->>viewModelScope: getString proxy_host_required or proxy_host_invalid
    viewModelScope->>events: send OnProxySaveError or OnProxyTestError
    deactivate viewModelScope
    events-->>UI: collect error event
Loading

Reviews (1): Last reviewed commit: "fix(tweaks): move suspend getString into..." | Re-trigger Greptile

@rainxchzed rainxchzed merged commit 8ee21c9 into main May 12, 2026
1 check passed
@rainxchzed rainxchzed deleted the fix/tweaks-suspend-getstring branch May 12, 2026 01:03
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.

1 participant