-
-
Notifications
You must be signed in to change notification settings - Fork 524
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
Give a proper message when trying to use an empty config file #1461
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome and thanks for the PR, @jonakeys. I had a slightly different intent for the behavior that I realize I hadn't made clear in the original issue: we still want jrnl to fail if the config can't be parsed, rather than falling back to the default config.
Also, the error message should appear when using a default config that's empty too. I added a test for that situation and fixed the other test so that you can take a test-driven approach to this if you'd like.
bdd-test doesn't add version if file is (and should stay) empty.
Thanks for your welcoming words and suggestions! I solved the issues and committed the changes. Had to add a condition to the test, because it would always automatically insert a version number and then the file wouldn't be empty. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Thank you!
Hi, I'm new to making commits so all feedback is welcome. Of course I put in my best effort.
This code adds a check for an empty config file, as specified by the issue #1420 . If an empty file is found, the programs prints a message and uses the default config, so the user won't be hindered in doing what he tried to do.
I added a bdd test, but couldn't get it to work properly. The test can't seem to find the new 'empty_file.yaml' config I created. Hopefully someone can help me out here.
Checklist
for the same issue.