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

Allowing osascript -l JavaScript as a JavaScript shebang #9337

Open
vitorgalvao opened this issue Jan 14, 2024 · 5 comments
Open

Allowing osascript -l JavaScript as a JavaScript shebang #9337

vitorgalvao opened this issue Jan 14, 2024 · 5 comments
Labels
A-core Area: Helix core improvements C-enhancement Category: Improvements

Comments

@vitorgalvao
Copy link
Contributor

JavaScript for Automation (JXA) is a companion/alternative to AppleScript available for default on macOS. One can use it in a script with the shebang:

#!/usr/bin/osascript -l JavaScript

Those are essentially JavaScript files and should be treated as such, but currently (if they don’t have an extension) I have to :set-language every time I open one.

Following the file in the repo, I edited languages.toml to have:

[[language]]
name = "javascript"
shebangs = ["node", "osascript -l JavaScript"]

But files with that shebang are still not parsed as JavaScript. However, it works if the configuration is instead:

[[language]]
name = "javascript"
shebangs = ["node", "osascript"]

Which isn’t quite right, because files without the -l JavaScript are “regular” AppleScript, which Helix doesn’t seem to support at all (but that’s a different issue).

So maybe a deeper change would be needed in Helix to support this use case? Is there a workaround I can use in the meantime?

@vitorgalvao vitorgalvao added the C-enhancement Category: Improvements label Jan 14, 2024
@the-mikedavis
Copy link
Member

This needs a change to the shebang capturing regex so that we also capture the arguments of the shebang:

const SHEBANG: &str = r"#!\s*(?:\S*[/\\](?:env\s+(?:\-\S+\s+)*)?)?([^\s\.\d]+)";

@the-mikedavis the-mikedavis added the A-core Area: Helix core improvements label Jan 16, 2024
@vitorgalvao
Copy link
Contributor Author

vitorgalvao commented Jan 16, 2024

Thank you for the context. It seems like an easy fix: removing the last \s. I can submit a PR for that and the languages.toml but the new question is if that is desirable? There might be a reason why the \s was added in the first place.

Or maybe you’d prefer I submit the PR anyway and the conversation continues from there?

@the-mikedavis
Copy link
Member

It looks like that regex originally came from #1001. Seems like it's custom so I don't think it should be a problem to change it

@WuerfelDev
Copy link
Contributor

When I changed the regex it didn't seem to work yet. I assume a bit more work in another place is required too
(Or I made a mistake)

@jw013
Copy link
Contributor

jw013 commented Jan 17, 2024

Some food for thought:

  1. sh -options is a very common shebang for shell scripts. How do we ensure all variations of sh -whatever continue to match against sh while supporting this case?
  2. osascript -l Javascript seems a bit rigid to me if an exact match is required. What if there are extra spaces or additional options?
  3. the current implementation relies on rejecting digits to e.g. match python3 input to a python configuration. It's fortunate that osascript has no number at the end, as supporting a hypothetical osascript3 -l Javascript shebang would generate additional complications.

The current shebang strategy seems to be:

  • have a universal SHEBANG regex that captures a match from the input
  • exact match this captured string against a set of configured fixed strings

I don't see a way for a single regex to cover all of the above cases but I wonder if maybe instead, the shebang matching could have two phases: the exe-name match that operates exactly how it currently does, and then a second step that matches on the arguments:

[[language]]
name = "javascript"
shebangs = ["node", { command = "osascript", arguments = ["-l Javascript"] }]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Helix core improvements C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

4 participants