Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run shellcheck and some more manual fixes #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Artoria2e5
Copy link

@Artoria2e5 Artoria2e5 commented May 28, 2020

  • ARGV is now an array. Should fix Quoted arguments get broken up #32.
  • Quotes and stuff, also in generated code.
  • Not using ... is a good idea because it effs up badly when you nest them. It's also in shellcheck.
  • Catch bad args in tickVars. And don't exit on help -- you are a function, act like one.

POSIX interaction, in case people want it later:

  • No arrays, but we aren't gonna do the transpiling anyways.
  • No +=, so it's gonna be dreadfully slow for long args. Remember the quadratic string append stuff in JS? Yeah it's exactly that.
  • No ${var/pat/sub}, so dreadfully slow again. Prefix and suffix trimming is available though.
  • No (( ... )), but [ "$(( ... ))" -ne 0 ] is the same. Except for ++.
  • No <<<, so I am effing myself up.

In addition, echo is a wack, but we can always roll our own with printf.

* ARGV is now an array. Should fix kristopolous#32.
* Quotes and stuff, also in generated code.
* Not using `...` is a good idea because it effs up badly when you nest them. It's also in shellcheck.
* Catch bad args in tickVars.

* * *

POSIX interaction, in case people want it later:
* No arrays, but we aren't gonna do the transpiling anyways.
* No +=, so it's gonna be dreadfully slow for long args. Remember the quadratic string append stuff in JS? Yeah it's exactly that.
* No ${var/pat/sub}, so dreadfully slow again. Prefix and suffix trimming is available though.
* No (( ... )), but `: $(( ))` is the same. Except for `++`.
@kristopolous
Copy link
Owner

Shell check is overly pedantic linting for people who don't know how to code. I'll look into the other stuff today, thanks

@kristopolous
Copy link
Owner

kristopolous commented May 29, 2020

Yeah sorry, this is a lot. I'll cherry pick some lines but I'm not going to accept all of it.

Also the idea wasn't to change the core but to have a derivative transpiler for posix

@kristopolous
Copy link
Owner

also you introduced a typo ... how about this ... package up the last commit (single liner) in a separate pr and I'll merge that ... there may be some shellcheck suggestions I missed ... but yeah ... I'll have to audit that line by line

@Artoria2e5
Copy link
Author

Quoting is not for overly pedantic folks; it's a way to explicitly tell the shell to not attempt to split and glob the filesystem at all. Doing as little as required is good behavior, not only because it might be a tidbit faster, but also because it means messing up less.

And how about your ARGV.

@kristopolous
Copy link
Owner

I'm really busy, thanks for the work, I'll look into it.

@XLTechie
Copy link
Contributor

XLTechie commented Jun 3, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quoted arguments get broken up
3 participants