Allow parameters for todo.sh default action

todotxt / todo.txt-cli

☑️ A simple and extensible shell script for managing your todo.txt file.

http://todotxt.org

GNU General Public License v3.0

5.55k stars 712 forks source link

Allow parameters for todo.sh default action #407

Open chrysle opened 1 year ago

chrysle commented 1 year ago

Before submitting a pull request, please make sure the following is done:

[x] Fork the repository and create your branch from master.
[x] If you've added code that should be tested, add tests!
[x] Ensure the test suite passes.
[x] Format your code with ShellCheck.
[x] Include a human-readable description of what the pull request is trying to accomplish.
[ ] Steps for the reviewer(s) on how they can manually QA the changes.
[ ] Have a fixes #XX reference to the issue that this pull request fixes.

This finishes leftover work from #159 That PR can be closed then.

I added a test, a docstring and also a note for TODOTXT_DEFAULT_ACTION to the config file.

inkarkat commented 1 year ago

Unfortunately, you're written the test to match the implementation, not to actually verify the specified behavior; and that actually was broken (good that we insisted on having tests). I've corrected both in my commit. It still doesn't handle default parameters that have spaces or other special characters; I'll add that later.

chrysle commented 1 year ago

Thanks for your help! What special parameters for example do you mean that should be tested?

chrysle commented 1 year ago

@inkarkat I would like to relieve you of implementing the tests if ou give me some hints.

inkarkat commented 1 year ago

Thanks for the review @chrysle; the original implementation was woefully incomplete and wrong, but only writing the tests showed what looked quite alright to a casual review. From my point of view this is now done, and we just have to wait for the merge of the huge backlog of open issues...

chrysle commented 1 year ago

Did you notice the test for macOS 10.15 succeeded? I wonder why it was always cancelled before.