CLI quiet mode - write database unlock errors to stderr #12403
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issue #12402 by ensuring that database unlock errors are written to stderr even when CLI commands are run in quiet mode. Previously, these password prompts and error messages were being suppressed completely in quiet mode, making debugging difficult.
Key Changes:
- Updated CLI utility to always write database unlock errors to stderr regardless of quiet mode setting
- Updated all test cases to expect password prompts on stderr even in quiet mode
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/cli/Utils.cpp | Modified to always write database unlock errors to stderr, removing quiet mode suppression |
| tests/TestCli.cpp | Updated test assertions to expect "Enter password to unlock" prompts on stderr in quiet mode tests |
| @@ -123,7 +123,7 @@ namespace Utils | |||
| const QString& yubiKeySlot, | |||
| bool quiet) | |||
There was a problem hiding this comment.
The code change removes the quiet mode logic but the quiet parameter is still present in the function signature. Consider removing the unused parameter or documenting why it's kept for future use to avoid confusion.
| bool quiet) | |
| const QString& yubiKeySlot) |
Copilot uses AI. Check for mistakes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #12403 +/- ##
===========================================
+ Coverage 64.19% 64.20% +0.01%
===========================================
Files 376 376
Lines 39371 39371
===========================================
+ Hits 25273 25275 +2
+ Misses 14098 14096 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
It is writing the unlock prompt to stderr right now by default 😅. When in quiet mode is just doesn't write the prompt at all, which doesn't make sense. The point of quiet mode is that nothing but data is written to stdout |
That was a bit unclear for me, because I thought only the errors should be written in quiet mode, not the password prompt. |
|
I'd like to get @phoerious opinion, too. |
|
IMHO, stdout should always be for anything that is payload data that you'd want to pipe somewhere or write to a file and stderr for anything that isn't, such as error and status messages. The prompt should be stderr as well. --quiet should suppress everything. |
Even errors? |
|
Errors should be communicated via return codes. What we could also do is -q for no output and -qq for no error output. |
|
That is a decent idea, I'll see what I can do here. |
keepassxc-cli show --quietsilently fails when providing a non-existent DB or invalid credentials #12402Testing strategy
Updated test cases to account for password prompt being written to stderr in quiet mode
Type of change