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

feat(boa): implements at method for string #1375

Merged
merged 3 commits into from
Jul 24, 2021
Merged

Conversation

neeldug
Copy link
Contributor

@neeldug neeldug commented Jun 29, 2021

This Pull Request relates to #13.

It changes the following:

  • implements at method for strings

@neeldug
Copy link
Contributor Author

neeldug commented Jun 29, 2021

Test262 conformance changes:

Test result master count PR count difference
Total 78,897 78,897 0
Passed 26,974 26,996 +22
Ignored 15,616 15,616 0
Failed 36,307 36,285 -22
Panics 0 0 0
Conformance 34.19% 34.22% +0.03%
Fixed tests:
test/built-ins/String/prototype/at/index-argument-tointeger.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/index-argument-tointeger.js (previously Failed)
test/built-ins/String/prototype/at/index-non-numeric-argument-tointeger-invalid.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/index-non-numeric-argument-tointeger-invalid.js (previously Failed)
test/built-ins/String/prototype/at/returns-code-unit.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/returns-code-unit.js (previously Failed)
test/built-ins/String/prototype/at/name.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/name.js (previously Failed)
test/built-ins/String/prototype/at/returns-item.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/returns-item.js (previously Failed)
test/built-ins/String/prototype/at/returns-item-relative-index.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/returns-item-relative-index.js (previously Failed)
test/built-ins/String/prototype/at/index-non-numeric-argument-tointeger.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/index-non-numeric-argument-tointeger.js (previously Failed)
test/built-ins/String/prototype/at/returns-undefined-for-out-of-range-index.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/returns-undefined-for-out-of-range-index.js (previously Failed)
test/built-ins/String/prototype/at/return-abrupt-from-this.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/return-abrupt-from-this.js (previously Failed)
test/built-ins/String/prototype/at/prop-desc.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/prop-desc.js (previously Failed)
test/built-ins/String/prototype/at/length.js [strict mode] (previously Failed)
test/built-ins/String/prototype/at/length.js (previously Failed)

@Razican
Copy link
Member

Razican commented Jun 30, 2021

Note that this doesn't close #13, since there are more methods left to implement.

@Razican Razican added builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request labels Jun 30, 2021
@Razican Razican added this to the v0.13.0 milestone Jun 30, 2021
Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. Check my comments. Note that this is a "proposal", and therefore not part of #13, and no other JS engine is implementing it. I want to know what @jasonwilliams thinks about adding these things to Boa.

boa/src/builtins/string/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/string/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/string/mod.rs Outdated Show resolved Hide resolved
@neeldug
Copy link
Contributor Author

neeldug commented Jun 30, 2021

Looks pretty good to me. Check my comments. Note that this is a "proposal", and therefore not part of #13, and no other JS engine is implementing it. I want to know what @jasonwilliams thinks about adding these things to Boa.

I mainly added it thinking it was failing the test262 suite, but I see what you mean.

@HalidOdat
Copy link
Member

Since it is in test262, its probably going to be added to the language eventualy, but maybe we should wait till then.

@jasonwilliams
Copy link
Member

Looks pretty good to me. Check my comments. Note that this is a "proposal", and therefore not part of #13, and no other JS engine is implementing it. I want to know what @jasonwilliams thinks about adding these things to Boa.

Its ok with me, I don't mind seeing proposals like this one being added as its quite likely to make it in

@Razican Razican merged commit 7c046ea into boa-dev:master Jul 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants