Skip to content

style: fix two clippy warnings#2237

Closed
hongqitai wants to merge 1 commit into
Hmbown:mainfrom
hongqitai:style/fix-clippy-warnings
Closed

style: fix two clippy warnings#2237
hongqitai wants to merge 1 commit into
Hmbown:mainfrom
hongqitai:style/fix-clippy-warnings

Conversation

@hongqitai
Copy link
Copy Markdown

@hongqitai hongqitai commented May 27, 2026

Summary

Run "cargo clippy" find two warnings, use "cargo clippy --fix" fix them.

Testing

  • [ x ] cargo fmt --all -- --check
  • [ x ] cargo clippy --workspace --all-targets --all-features
  • [ x ] cargo test --workspace --all-features

Checklist

  • [ x ] Updated docs or comments as needed
  • [ x ] Added or updated tests where relevant
  • [ x ] Verified TUI behavior manually if UI changes

Greptile Summary

This PR applies two mechanical clippy lint fixes generated by cargo clippy --fix, with no behavioral changes.

  • config.rs: Removes a redundant format!() call wrapping a plain string literal, replacing it with .to_string() directly on the literal (clippy::useless_format).
  • runtime_log.rs: Eta-reduces a closure |h| resolve(h) to pass the local closure resolve directly into and_then, eliminating a needless wrapper (clippy::redundant_closure).

Confidence Score: 5/5

Both changes are purely cosmetic clippy fixes with identical runtime behavior — safe to merge.

The two edits are exact semantic equivalents of the code they replace: removing a no-op format! wrapper and inlining a single-argument closure pass-through. Neither touches logic, data flow, or any externally visible behavior.

No files require special attention.

Important Files Changed

Filename Overview
crates/tui/src/commands/config.rs Replaces unnecessary format!("literal") with "literal".to_string() — a correct one-line clippy lint fix with no semantic change.
crates/tui/src/runtime_log.rs Eta-reduces `

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[set_config_value: base_url branch] --> B{persist?}
    B -- yes --> C[persist_root_string_key]
    C -- Ok --> D[CommandResult::message]
    C -- Err --> E["CommandResult::error(format!(...))"]
    B -- no --> F["CommandResult::error('base_url must be saved...'.to_string())"]

    G[log_directory] --> H{HOME env set?}
    H -- yes --> I[resolve HOME]
    H -- no --> J{USERPROFILE env set?}
    J -- yes --> K[resolve USERPROFILE]
    J -- no --> L["dirs::home_dir().and_then(resolve)"]
Loading

Reviews (1): Last reviewed commit: "style: fix two clippy warnings" | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies code in the TUI crate by refactoring a closure to a direct function reference in runtime_log.rs and replacing a format! macro with a string literal in config.rs. The reviewer noted that the .to_string() call on the string literal in config.rs is redundant and can be omitted to avoid an unnecessary allocation.

return CommandResult::error(format!(
"base_url must be saved with --save; client base URL is loaded from config on startup. Restart and re-open your session after saving."
));
return CommandResult::error("base_url must be saved with --save; client base URL is loaded from config on startup. Restart and re-open your session after saving.".to_string());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The .to_string() call is redundant here because CommandResult::error accepts a &str (or impl Into<String>) directly, as seen in other parts of this file (e.g., lines 34, 438, and 463). Passing the string literal directly is more idiomatic and avoids an unnecessary allocation.

            return CommandResult::error("base_url must be saved with --save; client base URL is loaded from config on startup. Restart and re-open your session after saving.");

Hmbown added a commit that referenced this pull request May 27, 2026
- #1937: Make DeepSeek V4 Pro 75% discount permanent (pricing.rs)
- #2237: Fix two clippy warnings (config.rs, runtime_log.rs)
- #1852: Add DEEPSEEK.md as project context file
- #1910: Suppress verbose CLI logging on Windows alt-screen
Copy link
Copy Markdown
Owner

@Hmbown Hmbown left a comment

Choose a reason for hiding this comment

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

Both clippy fixes are behavior-preserving: removing a useless on a string literal and eta-reducing a redundant closure. Clean.

APPROVE.

Copy link
Copy Markdown
Owner

@Hmbown Hmbown left a comment

Choose a reason for hiding this comment

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

Both clippy fixes are behavior-preserving: removing a useless format!() on a string literal and eta-reducing a redundant closure. APPROVE.

@Hmbown
Copy link
Copy Markdown
Owner

Hmbown commented May 27, 2026

Independent review: 3-way merge into origin/main (54151a4) is clean — exactly 2 files, +2/-4 as advertised:

crates/tui/src/commands/config.rs | 4 +---
crates/tui/src/runtime_log.rs     | 2 +-

Both fixes are present in PR #2256 (v0.8.48 crate consolidation):

  • runtime_log.rs:177dirs::home_dir().and_then(resolve)
  • commands/config.rs:476.to_string() in place of format!("…")

So this PR is functionally a fast-path of those two lines. If you want clippy-clean main before #2256 lands (e.g., to unblock other PR branches that fail clippy on rebase), merging this is the minimal patch. Otherwise it's safely orphaned when #2256 lands.

v0.8.48 (#2256) compatibility: orphans this work on merge of #2256 (same fixes already included).

@hongqitai
Copy link
Copy Markdown
Author

fixes are present in #2256 ,close this PR

@hongqitai hongqitai closed this May 27, 2026
@hongqitai hongqitai deleted the style/fix-clippy-warnings branch May 27, 2026 13:32
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