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

Remove nix and signal-hook dependencies #50

Merged
merged 3 commits into from
May 6, 2020

Conversation

malbarbo
Copy link
Contributor

@malbarbo malbarbo commented May 4, 2020

This reduces the dependencies from 9 to 2, the binary size (striped) from 515kb to 435kb (15%) and the build time from 9.36s to 2.96s , while it increases the line count from 1006 to 1012.

The editor in single thread, so I think the signal handling is ok, but it need to be reviewed.

@ilai-deutel
Copy link
Owner

Thanks a lot for this!
I will try to review this PR soon. In the meantime, would you mind adding documentation for the functions you added, especially register_winsize_change_signal_handler and set_windows_size_was_changed?

@malbarbo
Copy link
Contributor Author

malbarbo commented May 5, 2020

I will work on that.

Copy link
Owner

@ilai-deutel ilai-deutel left a comment

Choose a reason for hiding this comment

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

Great work!

Could please update the "Dependencies" sections of the README as well:

  • Removing mentions of nix and signal-hook
  • Removing the sentence "Safe wrappers are preferred...", which doesn't really apply anymore

src/unix.rs Outdated Show resolved Hide resolved
src/unix.rs Outdated Show resolved Hide resolved
src/unix.rs Outdated Show resolved Hide resolved
src/unix.rs Outdated Show resolved Hide resolved
src/unix.rs Outdated Show resolved Hide resolved
src/unix.rs Outdated Show resolved Hide resolved
Copy link
Owner

@ilai-deutel ilai-deutel left a comment

Choose a reason for hiding this comment

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

Thanks!

@ilai-deutel ilai-deutel merged commit 2b1ca9a into ilai-deutel:master May 6, 2020
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