[lmi] I will never understand 'test -n' [Was: Add script allowing to install any wxWidgets version from Git]

[Greg Chicares]

On 2018-04-07 11:20, Vadim Zeitlin wrote:
> On Sat, 7 Apr 2018 09:35:02 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2018-04-06 22:42, Vadim Zeitlin wrote:
[...]
> GC> Oh, wait: '-n" doesn't test whether it's null: it tests whether it's 
> nonnull.
> GC> So '-z' would be the appropriate alternative there. But because I 
> obviously
> GC> can't make sense of '-n' without extreme effort, let me ask instead 
> whether
> GC> the middle line could be replaced thus:
> GC> -   [ -n "$skip_update" ] || git checkout "$wx_commit_sha"
> GC> +   [ "$skip_update" = 1 ] || git checkout "$wx_commit_sha"
> 
>  Yes, sure. Just to explain how did I end up with the original version: I
> like using "-n", but I think that using both "-n" and "-z" may be
> confusing, so I wanted to avoid it but OTOH using "! -n" when "-z" exists
> didn't seem a good idea neither, so I used "$skip_update" != 1.

I had thought of using '! -z' instead of '-n', but that's ridiculous.
I can read '-z' as "zero-length", but I still read '-n' as "null" and
can't seem to understand it as "non-null".

But '[ "$x" = 1 ]' and '[ "$x" != 1 ]' seem comparatively lucid.

> GC> (2) Wouldn't the following be a pure refactoring (and simpler)?
> 
>  Sorry, not quite.
> 
> GC>     if [ $(git rev-parse HEAD) = "$wx_commit_sha" ]
> GC>     then
> GC> +       git checkout "$wx_commit_sha"
> 
>  This should go into the "else" branch: we only want to checkout this
> commit if HEAD does _not_ already point to it. And this is what the current
> test using "-n" does.

It sounds so reasonable in any other language.

> GC>         if ! git rev-parse --quiet --verify "$wx_commit_sha^{commit}" 
> >/dev/null
> GC>         then
> GC>             git fetch "$wx_git_url"
> GC>         fi
> GC> [or move the '+' line here if I still don't understand 'test -n']
> 
>  Yes, the test using "-n" means "true if $skip_update is not empty", so
> "git checkout" is executed only if this test is false.

Oh--after figuring out '-n', I was thinking '&&' instead of '||'.

> GC>     fi
> GC> 
> GC> -   [ -n "$skip_update" ] || git checkout "$wx_commit_sha"
> 
>  To summarize, it's true that we don't have to do this test at all here any
> more (sorry for leaving it in, I didn't realize it became unnecessary after
> the refactoring I did), but the checkout command must be done in the "else"
> branch, i.e. only if HEAD != wx_commit_sha.

Please tell me if I got it wrong in commit fdad3593.