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

Remove Unicode dependency to avoid large downloads for mathlib4 users #112

Closed

Conversation

eric-wieser
Copy link
Contributor

Until lake learns to not download this, I would claim that the benefit of marginally-nicer URLs is not worth the cost of having every mathlib4 user download a large archive of unicode data.

charInGeneralCategory c GeneralCategory.punctuation ||
charInGeneralCategory c GeneralCategory.separator ||
charInGeneralCategory c GeneralCategory.other
false -- TODO: restore the behavior described in the docstring
Copy link
Contributor Author

Choose a reason for hiding this comment

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

c == " " is probably a marginally better choice

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, go for it then 👍

@hargoniX
Copy link
Collaborator

hargoniX commented Feb 7, 2023

Do you want to make the mentioned change in the comment or shall I go ahead and merge it?

@eric-wieser
Copy link
Contributor Author

CI fails so I would recommend against merging this!

@hargoniX
Copy link
Collaborator

hargoniX commented Feb 7, 2023

Oh I was thinking this is just the usual CI fail from mathlib4 and std4 incompatability /o\

@hargoniX hargoniX force-pushed the main branch 4 times, most recently from b910c11 to 0c41523 Compare March 12, 2023 12:19
@fgdorais
Copy link
Contributor

Isn't this mostly fixed by 5f59fba

@hargoniX
Copy link
Collaborator

It's definitely a great improval yes, I'll close the PR for now.

@hargoniX hargoniX closed this Nov 24, 2023
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

3 participants