- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 467
 
Improve theme error informations #947
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
base: main
Are you sure you want to change the base?
Conversation
| 
           [APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: riquito The full list of commands accepted by this bot can be found here. 
Needs approval from an approver in each of these files:
 
      Approvers can indicate their approval by writing   | 
    
3b35e1f    to
    b76c3c2      
    Compare
  
    | 
           Is there anything I need to do to improve this PR?  | 
    
| 
           hi @riquito, sorry for the late reply, the PR looks good and I have started the CI. thanks for the contribution!  | 
    
          Codecov ReportAttention:  
 
 Additional details and impacted files@@            Coverage Diff             @@
##           master     #947      +/-   ##
==========================================
+ Coverage   85.76%   85.87%   +0.11%     
==========================================
  Files          51       51              
  Lines        5001     5026      +25     
==========================================
+ Hits         4289     4316      +27     
+ Misses        712      710       -2     ☔ View full report in Codecov by Sentry.  | 
    
I couldn't figure out why my local changes to the theme colors definition were not being picked up, so I went to the source.
Turned out my yaml file was using some improper fields.
This PR does:
color.themewas set to custom and nocolors.yaml(orcolors.yml) was foundContrary to before, if a color file definition exists and it cannot be parsed we immediately switch to the default (plus a warning). The previous logic was hiding format errors if there were other file extensions to try.
Some examples of the output (before regular lsd output):
if color.y(a)ml is found, but contains wrong data
if color.theme is set to "custom" in the config file, but no color.y(a)ml is present
Sidenotes:
cargo test. This was already happening withWarning: the 'themes' directory is deprecated, use 'colors.yaml' instead., I made it slightly worse. Perhaps in general warnings should be behind#[cfg(not(test))]or similar, didn't investigate inprint_erroreither, considered it out of scope.TODO
cargo fmt