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 i8.parse, f32.parse ... bool.parse to docs #174

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MaxGraey
Copy link
Member

No description provided.

@MaxGraey MaxGraey requested a review from dcodeIO August 26, 2022 14:51
@MaxGraey
Copy link
Member Author

@dcodeIO could you take a look please?

@dcodeIO
Copy link
Member

dcodeIO commented Aug 27, 2022

I would probably wait documenting these as long as there is nothing noteworthy these can do in addition, like hex float notation?

@MaxGraey
Copy link
Member Author

Ok, what is the best place e to put this into doc?

@MaxGraey
Copy link
Member Author

Can we merge it?

@dcodeIO
Copy link
Member

dcodeIO commented Jan 20, 2023

Discussion in Discord got me thinking about this again. Do I remember correctly that initially there was the thought of instead providing parseInt<T = f64>(str: string): T, and that you had preferred that originally?

@MaxGraey
Copy link
Member Author

Here description & motivation: AssemblyScript/assemblyscript#2465

@dcodeIO
Copy link
Member

dcodeIO commented Jan 20, 2023

Thanks, now I remember the context. Do you know what were the reasons for not considering to make parseInt and parseFloat generic instead? Seems that we ultimately ended up aliasing anyway.

@MaxGraey
Copy link
Member Author

Generic parseInt and parseFloat don't seem to solve this problem, because most likely we have to do something like parseInt<T = f64>(...) and parseFloat<T = f64>(...). Otherwise users will already have two compiled errors for these methods without explicitly specifying parameters. And I still suggest making the parseInt and parseFloat methods as deprecated

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