search

thinkerbot / ts

A shell test script
MIT License
59 stars 9 forks source link

Seemed the most maintainable code I've ever seen #26

Closed ghost closed 3 years ago

ghost commented 3 years ago

Tried to replace some test = and similar with case.

ThomasAdam commented 3 years ago

Hi @GH-TpaeFawzen

Other than a cosmetic change, is there a wider reason for this?

ghost commented 3 years ago

Hi @GH-TpaeFawzen

Other than a cosmetic change, is there a wider reason for this?

Hi @ThomasAdam, I'm sorry for replying late; my edit was to improve performance; test command (also known as square brackets) is not always built-in while case statement is always.

ThomasAdam commented 3 years ago

Hi @ThomasAdam, I'm sorry for replying late;

Hello @GH-TpaeFawzen -- that's OK -- I am sure most people are busy at the moment.

my edit was to improve performance; test command (also known as square brackets) is not always built-in while case statement is always.

Indeed, but that's very rare nowadays that test isn't a builtin; such shells are only ever found on really old Unixes, the likes of which ts wouldn't work with any way. test has been a builtin for pretty much all shells for a long time now. So I'm still seeing this change as stylistic only.

It's up to @thinkerbot whether he accepts this, but at least there's no functional change.

thinkerbot commented 3 years ago

Thanks @GH-TpaeFawzen for this and thanks @ThomasAdam . Apologies for the slow reply I got caught up in the holidays and lost track of this.

My guiding principle for this project was to make it only use things within the POSIX spec, in as much as possible. There are a few edges where I don't think I was able to pull it off, and ultimately the more important outcome is that it passes on as many distros as possible.

However, to that criteria, test is in the POSIX spec (see Shell & Utilities > Utilities > test). For that reason, and because I personally feel more confident in my use of if/then, I'm going to pass on this change. Your effort is appreciated though!