search

alacritty / vte

Parser for virtual terminal emulators
https://docs.rs/vte/
Apache License 2.0
242 stars 56 forks source link

adds test for UTF-8 parsing #2

Closed julienfitz closed 8 years ago

julienfitz commented 8 years ago
jwilm commented 8 years ago

This is exactly the right direction! Re: right place for the test; there's actually two crates here. There's the vte crate and the utf8parse crate. This test only involves the latter crate, so maybe it could all go under that tree?

julienfitz commented 8 years ago

No problem, can do! Thanks for the feedback, I will keep you posted on my progress.

jwilm commented 8 years ago

I'm happy to give as much feedback as you like! Really, I'm just super jazzed that you decided to tackle this issue. Thanks :smile:

julienfitz commented 8 years ago

Started trying my hand at further implementation, but not really sure what I'm doing 😛

Feel free to take a look at the most recent commit - I left comments about what I didn't understand. I can elaborate on any details if needed!

julienfitz commented 8 years ago

Added a new commit with some failed efforts. Commented with the errors particular to the lines. I am a little tired so my brain is not working very well at the moment, but I figured I'd show you where I was at! Maybe it will make more sense to me in the morning 😛

I'll squash all these when I'm done, just FYI 😄

julienfitz commented 8 years ago

Thanks for the feedback! Sorry that I haven't been able to get back to this yet this week. Should be able to get back to it in a few days.

jwilm commented 8 years ago

No worries! Take you time :)

julienfitz commented 8 years ago

Phew! Thanks for your patience. Okay, I think I'm a little further along, just having trouble figuring out exactly how to implement the utf8parse version. Any hints you might be able to offer on the most recent commit?

jwilm commented 8 years ago

Yeah! Take a peak at the "Parser::advance" method signature in the docs. Specifically, check how it wants the receiver to be passed. On mobile right now so can't get you a link easily, but I think there's one up this thread somewhere.

julienfitz commented 8 years ago

Sounds good! I'll let you know if I have any further questions. Thanks!

jwilm commented 8 years ago

I guess there wasn't a link in this thread yet. Parser::advance()

julienfitz commented 8 years ago

I think I might have done it?? Let me know what you think! If it is in fact done, I can squash these commits.

jwilm commented 8 years ago

Awesome! Let me grab your branch and give it a spin. Sadly I don't have travis set up yet because there was no tests up until now :flushed:

julienfitz commented 8 years ago

Okay, all done! Let me know if there's anything else I need to do. 😄

jwilm commented 8 years ago

This is awesome! Thank you thank you thank you :grin: