Skip to content

Conversation

@Roang-zero1
Copy link
Contributor

I've updated the error handling for invalid indexes.
The new version is compatible with the types I will be submitting shortly.

Error messages are now narrowed to the specific error.

Improve error feedback for better easier error handling.
@wooorm
Copy link
Member

wooorm commented Nov 17, 2019

I think this will change behaviour of e.g., falsey values that are not a number are passed through.
Is there a reason for changing the behaviour, other than error messages?

@Roang-zero1
Copy link
Contributor Author

I've updated the error handling to include falsey values.
The reason I updated the error handling is to be able to use typescript to add the types in #7 .
The original implementation is not typescript compatible, so I've updated it.

Copy link
Member

@wooorm wooorm left a comment

Choose a reason for hiding this comment

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

I think this’ll be good with a couple of changes!

@wooorm wooorm added 🏡 area/internal This affects the hidden internals 🙆 yes/confirmed This is confirmed and ready to be worked on 🧑 semver/major This is a change labels Dec 8, 2019
Roang-zero1 and others added 3 commits December 8, 2019 19:28
Co-Authored-By: Titus <tituswormer@gmail.com>
Co-Authored-By: Titus <tituswormer@gmail.com>
Co-Authored-By: Titus <tituswormer@gmail.com>
@Roang-zero1
Copy link
Contributor Author

Thanks for the suggestions, I've committed them.

@wooorm wooorm merged commit 1cb8d85 into syntax-tree:master Dec 8, 2019
@wooorm
Copy link
Member

wooorm commented Dec 8, 2019

Released!

@wooorm wooorm removed the 🙆 yes/confirmed This is confirmed and ready to be worked on label Mar 30, 2020
@wooorm wooorm added the 💪 phase/solved Post is done label Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏡 area/internal This affects the hidden internals 💪 phase/solved Post is done 🧑 semver/major This is a change

Development

Successfully merging this pull request may close these issues.

2 participants