Skip to content

Add documentation for GHC-21926 #514

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

Merged
merged 6 commits into from
Feb 22, 2025

Conversation

FPtje
Copy link
Contributor

@FPtje FPtje commented Jun 8, 2024

This is the card I drew during ZuriHac! 📇

This is quite a simple parser error, which can be hit when PackageImports is enabled. The error itself is quite clear already: don't put version numbers and non-alphanumeric characters in a package import.

I'm not sure how one would get into the non-alphanumeric character situation (besides an honest typo), but the version one is quite a fair mistake to make.

@FPtje FPtje requested a review from Bodigrim February 9, 2025 14:09
extension: PackageImports
---

Version number or non-alphanumeric character in the package. Alphanumeric characters are letters and numbers, as defined by `Data.Char.isAlphaNum`.
Copy link
Collaborator

@Bodigrim Bodigrim Feb 9, 2025

Choose a reason for hiding this comment

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

That's a bit incomplete: a package name can include dashes, even while Data.Char.isAlphaNum '-' = False.

The actual function GHC uses to validate package names is

looksLikePackageName = all (all isAlphaNum <&&> not . (all isDigit)) . split '-'

So probably something like "Each of dash-separated components of package name must consist of alphanumeric characters (as defined by Data.Char.isAlphaNum), at least one of which is not a digit". What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the late reply. Sounds good! Changed the code 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you push anything? I don't see any changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yikes, rookie mistake. Pushed as f1fde5c (#514).

Apologies for the inconvenience.

@FPtje FPtje requested a review from Bodigrim February 21, 2025 20:41
@Bodigrim Bodigrim merged commit 423843b into haskellfoundation:main Feb 22, 2025
2 checks passed
@Bodigrim
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants