5 ms·
FWIW I think ShellCheck is great and the state of the art, but Oil is partly (negatively) inspired by ShellCheck :) Somebody integrated ShellCheck into Google'
by chubot 7y ago
FWIW I think ShellCheck is great and the state of the art, but Oil is partly (negatively) inspired by ShellCheck :)
Somebody integrated ShellCheck into Google's code review system about four years ago, right before I left.
So the result was that every code review I sent with a shell script was filled with red squigglies -- "add double quotes here". Most code reviewers don't really know shell, but if they see red squigglies, they will say "fix this".
The double quotes are of course technically correct, but when you're writing a shell script in a subdir 8 levels deep in the repo that only operates on 3 files in that subdir, it's overkill and makes everything ugly.
In other words, it was the typical "static analysis has annoying false positives" issue.
Most of the shell scripts I write deal with "trusted" filenames -- i.e. those checked into the repo. If you want to deal with untrusted filenames, then there are a lot of ugly techniques to do that.
So Oil is basically meant to make the default thing the right thing, and the common thing the short thing (Huffman coding, as Larry Wall calls it).
- jonahx 7y ago> The double quotes are of course technically correct, but when you're writing a shell script in a subdir 8 levels deep in the repo that only operates on 3 files in that subdir, it's overkill and makes everything ugly. Once it becomes habit the cost is zero, though. And the alternative you're implicitly suggesting is: "Follow shellcheck except you are allowed to make an expert judgement and ignore it when you know it won't be dangerous and have determined that the code is unlikely to change in the future so that this exception will be safe going forward too." I think it's clear that this isn't a practical rule on a team with many people -- especially where they are not all experts in shell, which is 100% of teams of many people. Making the experts "add those annoying double quotes they don't really need in this case" is the far lesser of the two sins.
- acqq 7y agoExactly. If one "expert" is developing only for himself he can allow himself not to bother writing a script "clearly" (i.e. failing the programs that perform source checks). But as soon as the script is something that should be potentially sooner or later maintained by anybody else than the original author (and in any successful project that's practically sure) the only right policy is to produce the source code that's obvious and passes the source checks. An "expert" would have no problem to in no time adjust his three-line code to fulfill all the expectations of ShellCheck. If it takes more than that, that only proves that the checks are actually beneficial and even more important than the "expert" tells to himself: it proves that writing that specific code "cleanly" is actually hard for everybody.
- chubot 7y agoYeah I don't disagree with that. If you're working in a big group with some shell scripts, and want to get stuff done, use ShellCheck. I just find it annoying, hence the new shell :) There's a lot more to Oil, but that's definitely one of the surface annoyances I want to fix. Although zsh actually does fix that, and for some reason I've almost never seen a zsh script.
- fctorial 7y agoMany oh-my-zsh plugins are just collection of functions, which I think could be considered scripts. https://github.com/ohmyzsh/ohmyzsh/wiki/Plugins https://github.com/ohmyzsh/ohmyzsh/wiki/Plugins
- chubot 7y agoYes sorry I should have said "I rarely see a batch program in zsh". zsh is customized with zsh scripts for sure, and they are very extensive. I guess it's because zsh is not POSIX compatible by default.
- mehrdadn 7y agoIt's a burden to you only because you're used to not putting quotes in some cases. But to some of us who always put the quotes, it's not really about that at all, and it's not a subjective issue - missing them is literally wrong. It's like sprinkling random .split(" ") calls in your Python code for no reason, which you'd (hopefully?) never do. And when something is semantically so wrong, the elegance of its syntax compared to a correct solution becomes pretty much irrelevant.
- effie 7y agoQuotes are just a tool, not a religious requisite. Chubot is talking from experience, and he is right. Putting quotes on all shell expressions is a burden, because it's more keystrokes, and more characters on the display which make the source on display disorienting. When you use shell every day, you know where the quotes are needed and where they are just a noise. Use a tool - when it's needed.
- mehrdadn 7y agoYes, of course they're tools, but "religious requisite" is a pretty disingenuous representation of the argument I'm making here? Something being a tool doesn't mean its use or lack thereof can't be incorrect. Bikes are tools too, but you don't ride them on the highway, right? Quotes are tools that affect the correctness of your control flow quite vividly. Of course if there are multiple tools for solving the correctness issue you're welcome to choose any of them (and maybe the choice between the tools is something you can ascribe to religion), but that doesn't absolve you of the need to solve the issue itself. When the argument is that a piece of code is executing objectively wrong instructions (and ones that would be so obviously wrong in other languages!), that's pretty much the diametrical opposite of what you can casually dismiss as religious dogma...
- hkiely 7y agoWhich is best for simply catching typing errors? Shell check? I never saw the instillation instructions for oil.
- chubot 7y agoThe idea is that with Oil there will be better solutions to the problems that ShellCheck points out. As mentioned, the default should be right in Oil. The defaults are wrong in shell, and ShellCheck is meant to alert you of that. Oil should have a style linter / formatter, but if it's designed correctly it shouldn't need the analogue of ShellCheck, which is more about correctness.
- viraptor 7y ago> in the repo that only operates on 3 files in that subdir It does today. Will it change tomorrow? Will someone rename one of the files to include a space? It sounds like people saying "I don't need to escape this value in a query, it comes from a static list and it's safe." Yes - it's safe right now...
- 3xblah 7y ago"The double quotes are of course technically correct, but when ... it's overkill and makes everyrthing ugly." The only way you know this is being a person who routinely writes shell scripts. There is no substitute for knowing what you're doing. Try running shellcheck on configure scripts. Or try this curl -4O https://ftp.netbsd.org/pub/NetBSD/NetBSD-release-8/src/build.sh shellcheck build.sh Or try shellcheck on the build.sh from buildroot.org. These scripts are "safe" enough for hundreds or thousands of competent users to be running them every day.
- yyx 7y ago>all I get are false positives double quotes, mixing string and array, unused variables. Why is this normal?