Fix more issues identified through shellcheck

[a1346054]

More issues were identified through shellcheck and reading through the code.

Continues work on #345 but more help would be appreciated because I'm not too familiar with the codebase - more shellcheck issues remain to be fixed.

All tests pass.

[a1346054]

macOS users will have to update their bash versions, because the bash provided by apple is terribly out-of-date and will not be updated by apple anymore due to GPL licence concerns.

Bash 4.1, which provides the necessary functionality, was released in 2010, which was 11 years ago.

Updating is easy and most people who use the commandline (and hence todo.sh) will already have done so.

This was also one of the main reasons why the hashbang line was changed to '#!/usr/bin/env bash' earlier, so that the correct bash is started on macOS instead of the apple-provided one.

[inkarkat]

Thanks. That's again a whole lot of changes; most of it pretty straightforward. I wonder whether the consistent joining of for ...; do and if ...; then is worth it; this is purely cosmetic, but affects a lot of places.

Getting rid of the old Bash support on Mac OS would indeed be very useful; it's been a huge pain in the rear. I hope that your assessment that this is straightforward and most users would / can do it is correct. From the work on the mailing list, I know that at least for some people todo.sh is the only command-line application they use; some have installed Cygwin on Windows just for it.

I think the CI build on Mac OS would have to be upgraded first, so that we don't lose the test coverage on that platform.

[a1346054]

I'm using this guide https://google.github.io/styleguide/shellguide.html#loops which recommends those exact changes for the for ... do.

I'll revert the macOS changes and do a proper bash version check instead for that one read parameter.

[karbassi]

We could discuss dropping support for any version of Bash before 4.1 (2009-12-31). If we do, we'll release a major version upgrade to 3.0.0 for todo.txt-cli.

References:

• apple.com - Starting with macOS Catalina, your Mac uses zsh as the default login shell and interactive shell.
• thenextweb.com - Why does macOS Catalina use Zsh instead of Bash? Licensing

[a1346054]

If full compatibility with a locally installed updated bash version on a macOS were desired, then the hashbang

#!/bin/bash

would have to be changed to

#!/usr/bin/env bash

in every test file.

[a1346054]

I created two new branches that are ready for a pull request once this pull request is accepted.

The first one https://github.com/a1346054/todo.txt-cli/tree/macos_tests fixes issues with tests not running with the correct bash on macOS (and possibly elsewhere), as well as improves the general code consistency.

The second one https://github.com/a1346054/todo.txt-cli/tree/bash_syntax_tests further builds upon the first one by using more modern bash syntax, while still keeping backwards compatibility with older versions of bash.

All tests pass, as well as my own testing.

[karbassi]

@a1346054 can you convert all files to use tabs and add an editorconfig file to the project as well?

[a1346054]

Sure, I'll set it up. Any other preferences besides tabs over spaces?

[karbassi]

Sure, I'll set it up. Any other preferences besides tabs over spaces?

Great question. I think it would be great to use shfmt as the formatter. That way we have a "standard" to follow.

Could you set up shfmt in a new PR? Maybe set up sh-checker GitHub Action?

[a1346054]

I'd like to rework this PR and implement all the suggestions. Feel free to cherrypick the various commits though, or do whatever else is desired.

[karbassi]

I'd like to rework this PR and implement all the suggestions. Feel free to cherrypick the various commits though, or do whatever else is desired.

We'll wait for your rework. Thanks again!

[chrysle]

@a1346054 Is this still in progress or can it be closed?

[a1346054]

It can be closed for now, there are many pending PRs and shfmt/shellcheck can be setup at any point once the PRs are merged.