textmate / shellscript.tmbundle

TextMate support for ShellScript
43 stars 29 forks source link

Add shellcheck as an on-save handler for shell scripts. #26

Closed schinckel closed 8 years ago

schinckel commented 8 years ago

Marks the current file with warning/error/notes as applicable.

Includes documentation and binary.

schinckel commented 8 years ago

I've erred on the side of caution, and included the shellcheck binary, and all of the files that were installed by brew install shellcheck

schinckel commented 8 years ago

Oh, http://www.shellcheck.net/about.html

mkhl commented 8 years ago

Nice work, thanks!

That said, I’m not sure how I feel about that binary.

  1. I’m uncomfortable with user-contributed binaries on principle. For example, I can’t easily check that this binary corresponds to the upstream sources.
  2. My understanding of the GPL is that if we distribute the binary, we need to also distribute the source code, including and build scripts. I might be completely wrong though.
  3. That binary is Huge! :elephant:
  4. What’s the compatibility story of GHC-generated binaries across OS X versions?

As for the other files:

@sorbits @infininight Any advice regarding the binary? @schinckel I think this integration would make a great standalone bundle, how do you feel about that?

schinckel commented 8 years ago

Yeah, a standalone bundle could be best.

One other alternative is to use an environment variable for the command location (it's brew-installable). However, having a single install process would be simpler.

mkhl commented 8 years ago

One other alternative is to use an environment variable for the command location (it's brew-installable).

Right, that way we could leave this command in the bundle and leave installation to the user. The C bundle provides a nice example for this (see the requiredCommands key in ~/Library/Application\ Support/TextMate/Managed/Bundles/C.tmbundle/Commands/Reformat\ Document.tmCommand).

When doing that though I’d prefer this to be a “Validate Style” kind of command (the common shortcut for those is ⌃⇧V) so users don’t have to install GHC just to save shell scripts.

Which way do you prefer?

sorbits commented 8 years ago

Using the requiredCommands (without putting the binary in the bundle) and putting it on ⌃⇧V sounds like the best way.

You can include instructions about how to set it up to run on save.

And as for requiredCommands this can be set to look in common install locations (like homebrew’s), so all the user need to do is run brew install shellcheck.

MikeMcQuaid commented 8 years ago

@schinckel This feels like something that https://github.com/sxtxixtxcxh/validate-on-save.tmbundle may be interested in?

mkhl commented 8 years ago

This change seems to have found its way into https://github.com/sxtxixtxcxh/validate-on-save.tmbundle. Feel free to reopen if you want to discuss this further.