[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lmi] [lmi-commits] master 29b03dc 4/5: Improve portability of shell
From: |
Vadim Zeitlin |
Subject: |
Re: [lmi] [lmi-commits] master 29b03dc 4/5: Improve portability of shell scripts |
Date: |
Sat, 7 Apr 2018 20:18:50 +0200 |
On Sat, 7 Apr 2018 15:33:52 +0000 Greg Chicares <address@hidden> wrote:
GC> On 2018-04-07 11:01, Vadim Zeitlin wrote:
[...]
GC> > If not, I'd have to update the PR 80 as it contains some more unquoted
GC> > variable expansions (all of which are still safe though).
Sorry, I had written this before looking at the later changes, so this
part was already out of date when I wrote it.
GC> I'm trying to ensure that you don't have to update any PR; thus:
GC>
GC> f611bc98 Use relative URLs for wxWidgets submodules in install script
GC> ee4adeaf Roll back some changes to avoid cherry-pick conflict
GC>
GC> where f611bc98 is PR 80 without any modification.
GC>
GC> My goal is to avoid creating needless extra work for you when I
GC> cherry-pick PRs like that one, so please let me know if I'm not
GC> achieving that goal.
You definitely are, I just hadn't realized that you already had, sorry
again -- and thanks for taking care of it.
GC> I believe I understand some of them fully: '$()' vs. '``', for example.
FWIW I know that $() is preferred and `` deprecated but personally I still
prefer the latter because it's
(a) shorter (yes, a whole extra character)
(b) seems to stand out more to me as $() is quite similar to ${}
But, of course, $() has some advantages too, i.e. (b) could also be
interpreted as it being more consistent and the counterpart of (a) is that
$() allows nesting, unlike `` (although I'd argue that it might be better
to use a temporary variable instead of nesting command substitutions...).
I'm not aware of any other important advantages of $() but I understand
that those may already be enough to prefer it.
GC> But I really have no idea when double quotes are
GC> {necessary, legal-but-less-readable, harmful, ...}
GC> especially the middle case, as above:
GC> - git clone $wx_git_url ${wx_dir##*/}
GC> + git clone "$wx_git_url" ${wx_dir##*/}
GC> How can we be certain that a variable like $wx_git_url contains no
GC> whitespace? On a corporate msw machine where we create a mirror at
GC> $HOME/MyMirror/
GC> couldn't that expand to
GC> Corporate Employees/John Smith/MyMirror
GC> ?
OK, I admit that it would fail in this case. When I wrote that wx_git_url
would never contain any whitespace, it was because using paths with spaces
in them with shell or any other Unix tools is painful, precisely because it
requires a lot of quoting, and so I don't know of anybody who would name a
path meant to be used with Unix-like utility, such as Git, on purpose. But
I guess it can still happen accidentally and quoting is indeed required in
this case.
Of course, wx_commit_sha is really never going to have spaces in it, but
now I don't even know if it's worth removing the quotes around it (and
disabling the shellcheck warning about it) or keeping them for consistency,
even if they're not really required... If you prefer to keep them, I'll try
to remember to use them in the future and run my changes through shellcheck
before submitting them.
Regards,
VZ