refactor: print progress only when terminal #2276
Conversation
Also login failed message improved
There was a problem hiding this comment.
Pull request overview
This pull request refactors progress output to only display when a terminal is present and removes duplicate login failure messages from the error flow.
Changes:
- Modified progress indicator printing to check for
$ENV{TERM}presence before outputting - Updated login error handling to suppress duplicate "Login failed" error messages
- Removed password from default admin user creation log message
- Added database index on
id_bundlecolumn indomains_bundletable
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| script/rvd_front | Filters out duplicate "Login failed" error messages in the login error handling |
| lib/Ravada/Auth/SQL.pm | Added newline character to "Login failed" error message for cleaner output |
| lib/Ravada.pm | Added $ENV{TERM} checks to all progress print statements, removed password from admin user creation warning, and added index on id_bundle column |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (defined $login && defined $password) { | ||
| eval { $auth_ok = Ravada::Auth::login($login, $password)}; | ||
| push @error,($@) if $@; | ||
| push @error,($@) if $@ && $@ !~ /Login failed/; |
There was a problem hiding this comment.
The regex pattern $@ !~ /Login failed/ is case-sensitive and could fail to match if the error message capitalization changes. Additionally, the error message in lib/Ravada/Auth/SQL.pm now ends with a newline character, which could affect the pattern matching if not accounted for. Consider using a case-insensitive match with the /i flag or anchoring the pattern appropriately to ensure reliable matching.
| push @error,($@) if $@ && $@ !~ /Login failed/; | |
| push @error,($@) if $@ && $@ !~ /Login failed/i; |
Also do not show duplicate login failed message