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

Accept shebangs in code blocks #46

Open
brandonkal opened this issue Mar 17, 2020 · 11 comments
Open

Accept shebangs in code blocks #46

brandonkal opened this issue Mar 17, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@brandonkal
Copy link

A code block that starts with a shebang should be interpretted as an executable file. This enables mask to support any interpreter.

Ref: https://github.com/casey/just#writing-recipes-in-other-languages

@jacobdeichert jacobdeichert added the enhancement New feature or request label Mar 17, 2020
@jacobdeichert
Copy link
Owner

I thought about this before, but it would incur a change in behavior and tiny performance penalty.

Right now, mask just passes source strings to runtimes via arguments (most commonly -c). This means that mask doesn't have to write to a temp file and execute it at all, which is a bit more work.

AFAIK shebangs are only effective when executing a file. Since mask doesn't write temp files to execute, this isn't possible yet.

Mask already supports the most common runtimes and it's easy to add more if needed. I'm unsure how important adding shebang support really is at this point.

@brandonkal
Copy link
Author

It would only be a tiny performance penalty on the first run.

For example, the Deno runtime compiles and type-checks code before executing. If it is passed a file, it can cache compilation for that file path, significantly speeding up subsequent runs. Unfortunately booting up the typescript compiler for deno is measurably slow for the first file. So for Deno TypeScript to work with a good user experience, writing to files would be required. At that point, general shebang support makes sense to me.

@feep
Copy link

feep commented Aug 13, 2020

Adding hashbang and pound-bang keywords to make issue more searchable.

@brandonkal
Copy link
Author

Just an FYI that shebangs are currently supported in my fork: https://github.com/brandonkal/inkjet

@texastoland
Copy link

texastoland commented Aug 7, 2021

No PR? This was my request too! Even if easy to add runtimes (you already support the most popular ones) some may be too niche. Use cases:

@jacobdeichert
Copy link
Owner

Yeah, I do think adding support for shebangs will be useful.

My concern above regarding the tiny perf hit to write to a temp file can be avoided by checking if the code block starts with a shebang and only write+execute a temp file in that case.

@texastoland
Copy link

After I commented I realized 1 of your biggest DX improvements over Maid is injecting arguments by name. Shebang might be nice for corner cases like awk or transpiled JS but it'd be sad to forgo that feature in .NET or Deno. I wonder if there's a way to query GitHub for any popular ones not mentioned here 🤔

@jacobdeichert
Copy link
Owner

Since argument injection is just setting environment variables, I would assume that executing a temp file which contains a shebang would still be able to use those set env vars. Though, I could be wrong... haven't tried this before 😅

@texastoland
Copy link

texastoland commented Aug 8, 2021

Ah I see that in add_utility_variables now. I imagined it was something trickier. Now I understand about the temp file too. It looks like there are minor bugs with the current command config though:

  1. PowerShell is cross-platform but it's configured for Windows.
  2. Your fallback assumes the target shell is the same as the calling shell. That's likely not the case. Even the differences between zsh (default on new Macs) and bash (let alone fish or Nu) are enough to blow up a script (particularly due to quoting differences).

I think a more general and flexible solution could be a cascading lookup for commands:

  1. Some TOML/JSON/whatever config file.
  2. Override with a user config in $XDG_CONFIG_HOME (or the Windows equivalent) …
  3. And/or in a parent directory of the maskfile.
  4. Either (but I prefer the next option) override with shebang (if included) using a temp file ...
  5. Or with a custom command (if included) following the lang (inspired by ReadMe-Flavored Markdown).

Configs:

Default:

[deno]
langs = ['ts', 'typescript']
[deno.command]
# WARNING: eval implies --allow-all
sh    = 'deno eval'
# in this case same on Windows
ps    = 'deno eval'
cmd   = 'deno eval'

[node]
...

User:

[deno]
command = '''deno eval $SCRIPT --print'''

maskfile.md

Default:

```ts
"this is denoooo"
```

Shebang (temp file):

```ts
#!/usr/bin/env deno run --allow-read --allow-write
console.log("this is denoooo with permissions")
```

Inline command (no temp file and shorter):

```ts ts-node -p
"this is noooode"
```

@jacobdeichert
Copy link
Owner

PowerShell is cross-platform but it's configured for Windows.

Yeah, Windows and PowerShell support were added together early last year. If someone opens an issue for PowerShell support on linux/macOS, it should be a quick change.


Your fallback assumes the target shell is the same as the calling shell.

Hmmm, I'm not quite sure I follow. Are you talking about this fallback block below? I think this actually relies on the code block lang code to determine the executor... so if someone uses "fish" as the lang code, that will be the shell that is invoked.

https://github.com/jakedeichert/mask/blob/ab46b568a21731a58507367ccd6b869579e8a308/mask/src/executor.rs#L63-L68


I think a more general and flexible solution could be a cascading lookup for commands

Yeah, I'll need to think about this a bit more. I feel like adding shebang support is enough to cover 95% of use cases without having to resort to introducing a config file. FWIW I've tried to keep mask super minimal and only require a single maskfile.md to operate. Supporting a user-specific config file for mask would result in maskfiles being less portable and more dependent on the individual users sharing the exact same config setup.

@texastoland
Copy link

I think this actually relies on the code block lang code to determine the executor

My bad I misread it.

If someone opens an issue

Seems silly okay ...

Supporting a user-specific config file

My main point is you save yourself work making it a config file:

  1. It could at least be a config file in this project since it isn't doing anything dynamic in code.
  2. You wouldn't have to maintain corner cases (examples above) if that config file could be overriden.
  3. If the config file is in the same project it wouldn't be machine-specific. That issue seems inescapable though. You can make reasonable assumptions like a POSIX-compatible shell (using WSL on Windows or Yarn's shell emulator) or maybe even some version of Node, but zsh, new ESM Node, and Deno are all examples that would have to be enforced by a Docker file or something anyway. It occurred to me for Node projects we're particularly beholden to Node alternatives like Maid because they're installable as dependencies.
  4. Yes shebang covers the same ground but more verbosely (example above) and more convoluted because it's expecting a file instead of string or stdin.

Neat project I'll circle back when I have a concrete use case ✌🏼

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

No branches or pull requests

4 participants