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

Add support for extra Unix shebangs #1163

Merged
merged 4 commits into from
May 19, 2021

Conversation

TristanCacqueray
Copy link
Collaborator

This change enables using nix-shell multiline shebangs in dhall files.

@TristanCacqueray
Copy link
Collaborator Author

@@ -1918,7 +1918,10 @@ nonEmptyListLiteral = do
return (NonEmptyList (e0 :| es))

shebang :: Parser ()
shebang = do "#!"; many notEndOfLine; endOfLine; return ()
shebang = do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of changing the implementation of shebang, I'd suggest changing the implementation of completeExpression from optional shebang to many shebang. The reason I suggest this is because it would more closely correspond to the ABNF grammar

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That also looks much simpler

@@ -1,2 +1,3 @@
#!/usr/bin/env -S dhall text --file
#! extra bang
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, rather than change this test, I'd suggest adding a new test for the nix-shell example that motivated this change

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks for the review!

This change enables using nix-shell multiline shebangs in dhall files.
This change fixes the scripts path in the CONTRIBUTING.md file.
Copy link
Collaborator

@basile-henry basile-henry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

tests/parser/success/unit/ShebangNixA.dhall Outdated Show resolved Hide resolved
@Gabriella439 Gabriella439 merged commit 2547fe7 into dhall-lang:master May 19, 2021
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.

None yet

4 participants