Skip to content

Fix scripts without an .hs or .lhs extension #7917

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 25, 2022
Merged

Fix scripts without an .hs or .lhs extension #7917

merged 2 commits into from
Jan 25, 2022

Conversation

bacchanalia
Copy link
Collaborator

@bacchanalia bacchanalia commented Jan 20, 2022 •
edited
Loading

  • Pass such scripts with: -x hs script
  • Disable extension check for scripts
  • Disable running preprocessors for scripts because they're based on
    extension
  • Add test case for script with no extension

Resolves #7910

@jneira
Copy link
Member

jneira commented Jan 20, 2022

Many thanks for the quick fix

Disable running preprocessors for scripts because they're based on extension

What would be the practical implications? doesnt sound good at first

@bacchanalia
Copy link
Collaborator Author

@jneira It just means that the script has to be a plain Haskell file, not GreenCard, c2hs, hsc2hs, happy, alex and cpphs (to quote from Distribution.Simple.PreProcess). This was also effectively the case before #7851, because the source file was copied to a Main.hs within the package. It would be possible to loosen this and allow those file types to be preprocessed but I'm not sure that's something useful to support. It would also be a change from previous behavior.

@bacchanalia bacchanalia linked an issue Jan 20, 2022 that may be closed by this pull request
allow scripts to have any name (not just Main.hs) #5702
Closed
jneira
jneira reviewed Jan 21, 2022
View reviewed changes
@bacchanalia
Copy link
Collaborator Author

bacchanalia commented Jan 24, 2022 •
edited
Loading

Scripts with spaces in the name are not being accepted. I think it makes sense to include the fix for that as part of this PR since it's fixing the bug "allow scripts to have any name". Investigating now.

Mikolaj
Mikolaj approved these changes Jan 24, 2022
View reviewed changes
Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

LGTM

@bacchanalia
Copy link
Collaborator Author

@Mikolaj should have waited for the tests to run

@Mikolaj
Copy link
Member

Mikolaj commented Jan 24, 2022

@Mikolaj should have waited for the tests to run

I knew they would pass. Eventually.

jneira
jneira reviewed Jan 24, 2022
View reviewed changes
jneira
jneira approved these changes Jan 25, 2022
View reviewed changes
Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

lgtm, many thanks for fixing it so fast
i checked this in windows 7 with weird names like sayñ¿Ç#$ä 2èáê and worked fine 👍

@bacchanalia bacchanalia added the merge me Tell Mergify Bot to merge label Jan 25, 2022
@bacchanalia
Fix scripts without an .hs or .lhs extension
6fdc940 
* Pass such scripts with: -x hs script
* Disable extension check for scripts
* Disable running preprocessors for scripts because they're based on
  extension
* Add test case for script with no extension

Resolves haskell#7910
@bacchanalia
Allow script names to contain whitespace and ':'s
46987a9
@mergify mergify bot merged commit a3bc167 into haskell:master Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jneira jneira jneira approved these changes

@Mikolaj Mikolaj Mikolaj approved these changes

Assignees
No one assigned
Labels
merge me Tell Mergify Bot to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Scripts without an .hs or .lhs extension no longer work allow scripts to have any name (not just Main.hs)
3 participants
@bacchanalia @jneira @Mikolaj