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

Clean warnings #1133

Merged
merged 7 commits into from
Jan 30, 2025
Merged

Clean warnings #1133

merged 7 commits into from
Jan 30, 2025

Conversation

yaxu
Copy link
Member

@yaxu yaxu commented Jan 30, 2025

No description provided.

@yaxu
Copy link
Member Author

yaxu commented Jan 30, 2025

@sss-create Just checking you're not already working on this?

@yaxu yaxu merged commit 327d995 into dev Jan 30, 2025
36 checks passed
@yaxu
Copy link
Member Author

yaxu commented Jan 30, 2025

That was easier than I thought!

@yaxu yaxu deleted the clean-warnings branch January 30, 2025 15:22
@sss-create
Copy link
Collaborator

sss-create commented Jan 30, 2025

nice! I started working on this in my fork but did not get too far yet. Did you replace the head/tail functions yet? I thought we could maybe use Debug.Trace to message the user in case of empty lists. What is your take on this?

@yaxu
Copy link
Member Author

yaxu commented Jan 30, 2025

Hm I'm not seeing any warnings about head/tail functions.. How are you getting them?

@yaxu
Copy link
Member Author

yaxu commented Jan 30, 2025

Ah I see them with the latest rather than the recommended ghc, along with some other new warnings. I don't have any strong opinions about head/tail really.

@sss-create
Copy link
Collaborator

head/tail warnings are annoying imo. However a strategy to get rid of them is needed. On one hand, the exception head and tail may throw are good to inform the user about a mistake when e.g. passing an empty list. OTOH, it's a scary error message. Same goes for '!!'.

So we'd have to decide what we want to replace head/tail with:
Data.uncons, pattern matching, listToMaybe, NonEmpty or whatever.

@yaxu
Copy link
Member Author

yaxu commented Feb 1, 2025

I think tail can probably be replaced with drop 1 in most/all cases. Maybe we don't need to warn the user. There might be some cases where it could be useful but it might result in spamming the console if it's triggered each time a pattern is queried.

@sss-create
Copy link
Collaborator

I'm undecided but I'd be in favor of removing head/tail from the code base since Tidal's install procedure exposes the user to GHC's warnings. No warning/exception when evaluating a pattern could be irritating though.
do i use the apostrophs correctly? 👀

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.

2 participants