Skip to content
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

Fix literal flag not being recognized from config file #935

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

Akmadan23
Copy link
Contributor

@Akmadan23 Akmadan23 commented Oct 16, 2023

The literal option was not correctly recognized from config file due to Literal::from_cli returning Some(Literal(false)) instead of None, so Literal::from_config would never have been executed.


TODO

  • Use cargo fmt
  • Add necessary tests
  • Update default config/theme in README (not applicable)
  • Update man page at lsd/doc/lsd.md (not applicable)

@muniu-bot
Copy link

muniu-bot bot commented Oct 16, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Akmadan23

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Akmadan23
Copy link
Contributor Author

Sorry @zwpaper, I noticed you have been active in merging PRs recently, could you please take a look at this? It's a very simple fix to a real problem.

@zwpaper
Copy link
Member

zwpaper commented Nov 10, 2023

hi @Akmadan23 , sorry for the late reply, I have started the test and will take a deeper look into it tomorrow, it looks good at a glance 😃

@zwpaper
Copy link
Member

zwpaper commented Nov 12, 2023

hi @Akmadan23, I have checked your fix, and it works like a charm! thanks for the contribution!

please rebase your code onto master, so that CI can pass, and then we should be good to go

@zwpaper zwpaper added this to the v1.1.0 milestone Nov 12, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6f8e095) 85.76% compared to head (0231b9b) 85.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #935      +/-   ##
==========================================
- Coverage   85.76%   85.75%   -0.01%     
==========================================
  Files          51       51              
  Lines        5001     4998       -3     
==========================================
- Hits         4289     4286       -3     
  Misses        712      712              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Akmadan23
Copy link
Contributor Author

Now everything should be good to go

@zwpaper zwpaper merged commit 5d857b5 into lsd-rs:master Nov 15, 2023
20 checks passed
@zwpaper
Copy link
Member

zwpaper commented Nov 15, 2023

thanks for the contribution!

@Akmadan23 Akmadan23 deleted the fix-literal branch November 15, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants