-
Notifications
You must be signed in to change notification settings - Fork 70
Rewrite for better code structure, lots of refactoring, use tokio/async #138
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The implementation was quite bad, and I never got around implementing it better. Remove all happy path logs, comment-out exception logs.
- Remove statusline config parameter -- it's currently not supported. - Formatting - Remove "development" section, crate a file for "contributing" (currently empty)
Owner
Author
|
There are a few remaining issues in libtiny_client before merging this, see #136 . |
Only remaning blockign delay is after address name is resolved and we're trying to establish a TCP (and maybe TLS too) connection. I think this one can wait though ...
This was referenced Sep 30, 2019
Closed
osa1
added a commit
that referenced
this pull request
Oct 3, 2019
Before #138 a SIGWINCH would cause mio's poll() to fail, and on every error we'd check GOT_SIGWINCH variable (which is set on SIGWINCH). After switching to tokio this stopped working (see #142 for more details). SIGWINCH handling is now done via tokio_net's signal handling facilities, in TUI instead of term_input. Fixes #142
|
This helps rust-lang/rust#65819 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR basically rewrites the whole app except the TUI parts. The main motivation is to make the code easier to work with.
Main changes are:
The code is now split into a few crates, with few inter-dependencies. Dependencies between the new crates look like this:
The main crate (tiny, which previously implemented pretty much the whole program) now does a few things:
conn.rsin 510 lines.ui.rsin 159 lines./connect,/away,/joinetc. (UI-related commands are now handled in the UI libraries, e.g. libtiny_tui)Other than these it just glues the libraries together: initializes TUI, clients, logger etc.
libtiny_wire implements parsing and generating IRC messages. Only difference from the previous
wiremodule isQUITandNICKmessages now have a field for channels of the user who quit or changed their nick. libtiny_client updates these fields, to be used by the main crate.libtiny_client implements the IRC driver. It handles reconnections, nick selection, authentication, ping/pongs, sending messages etc. Users interact with a client using a the
Clientmethods and receives IRC events from a tokiompscchannel.Note that once created, a client is only destroyed when the user wants to close the connection to the server -- that is, clients are not destroyed on connection errors.
A client now maintains a list of nicks for every channel joined, and updates
chansfields ofQUITandNICKmessages before sending them to the user, so that UIs or the logger doesn't have to maintain a list of channels a user is in.libtiny_ui defines a trait for UIs and a few utilities for combining two UIs (see below).
(This will be used for a GUI in the future, see the placeholder libtiny_gui crate)
libtiny_tui is the good old TUI implementation, which now implements the UI trait from libtiny_ui. The top-level is refactored to support use with tokio -- most notably it implements
Cloneand methods now require&self(instead of&mut selfas before).Similar to
Client(libtiny_client), TUI events are returned to the user using a mpsc channel.Other than that the code is the same as before.
libtiny_logger implements logging. Logger is implemented as a UI, so in the use site we don't actually do any logging, we just combine the main UI (the TUI) with a logger (if logging is enabled by the user), and then rest of the code just updates the UI as usual. This is quite convenient in the use sites.
For debug logging we now use
env_logger. By default debug logging is disabled. When enabled logging is done to stderr. Because some of the dependencies (e.g. tokio) also use the standardlogcrate it's a good idea to limit the logging scope, using something like:Breaking changes
The only breaking change is that the
/reloadcommand is currently disabled. This is because the command is somewhat special in that it requires parsing the config file in the UI, and I couldn't find a good way to implement this yet.Other than that this is pretty much just refactoring. Only user-facing change is the new logger (it's a good idea to remove the old logs directory before starting using new tiny, but it's not necessary, logging will still work).
Trivia
git clean -xfd: before 17.90s, after 26.96s (+9.06s, +50%)git clean -xfd: before 39s, after 1m11s (+32s, +82%)cargo metadata --format-version=1): before 118, after 168 (+50, +42%)Fixes #100
Fixes #94
Fixes #3
Fixes #56
Fixes #132