8000 Refactoring ansi/iterator by zhiburt · Pull Request #1191 · dandavison/delta · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Refactoring ansi/iterator #1191

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 15, 2022
Merged

Refactoring ansi/iterator #1191

merged 1 commit into from
Sep 15, 2022

Conversation

zhiburt
Copy link
Contributor
@zhiburt zhiburt commented Sep 14, 2022 •
edited
Loading

Hi there
As I was looking through the file and I decided to make it a bit cleaner.

No actual changes are supposed to be done.
Except her it was unsound that we are not doing anything with old element but I decided to make it explicit None.

https://github.com/zhiburt/delta/blob/22ff23d588a9b483b9457f8df6b414f59122b7e1/src/ansi/iterator.rs#L134

Take care

@zhiburt
Refactoring ansi/iterator
22ff23d 
Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>
@dandavison
Copy link
Owner

Thanks a lot! I really like the look of this. I'll look properly this evening.

@dandavison
Copy link
Owner

I agree with you that that code needed cleaning up!

Incidentally, there's some ansi_style stuff in there which I think may be appropriate for delta but probably we should make it optional in the crate. If it improves benchmarking times to remove it we may want to do that sooner rather than later.

@dandavison
Copy link
Owner

This is much, much nicer than the code it's replacing. Thanks a lot. I believe it also benchmarks slightly faster (3-5%? I was using make benchmark). Do you have time to also apply these changes to https://github.com/dandavison/vte-ansi-iterator, so that your authorship is attached to them?

@dandavison dandavison merged commit 1193d54 into dandavison:master Sep 15, 2022
dandavison pushed a commit to dandavison/vte-ansi-tools that referenced this pull request Sep 15, 2022
@zhiburt
Update iterator.rs (#3)
30cba5a 
Applies changes from dandavison/delta#1191
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zhiburt @dandavison
0