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

Refactor data flow and terminal library #117

Merged
merged 72 commits into from Feb 7, 2016
Merged

Refactor data flow and terminal library #117

merged 72 commits into from Feb 7, 2016

Conversation

@hannesm
Copy link
Owner

@hannesm hannesm commented Feb 6, 2016

The data flow design was slightly broken, a global Cli_state.state was passed around and mutated, by both user input, the rendering loop, and network input. Error handling was done at the beginning of the rendering loop. Also, by using (outdated) Lambda_term/Zed/Camomile libraries, the width of unicode characters (can be 2 columns) lead to UI problems (motivated by 茶 as status line, initially reported by @mirko).

Now, there is a rewrite: the main loop renders the state, receives a state -> state Lwt.t function from a Lwt_mvar, and continues (unless quit was executed).

Terminal input handling is done in a separate task (to have SMP question support, where both a question and a shared secret needs to be entered, another Lwt_mvar is used (plus a global bool ref, bad bad bad).

Commands, navigation, also network input puts something into the state -> state Lwt.t mvar. Thus, there's a single (immutable!) state (NB: the Contact.contacts still uses a Hashtbl, but should be straightforward to use a Map.t now). To comply with this design, the network interaction needed slight improvement. Error handling is now done while executing the action, and while rendering (instead of having every potentially failing network send wrapped in a try_lwt). Once a disconnect happens, a reconnect timer is started (via Lwt.async from the main loop)!

The rendering now uses the notty library, which exposes combinators for rendering text. The rendering code is hugely improved (and much more readable), additionally, there is no need anymore to compute width and height manually.

There might be some regression (such as certain key bindings do not work, C-, C-, mark/yank/paste, undo/redo), also smart line wrapping (find whitespace within the last 10 chars) is not implemented yet.

@hannesm
Copy link
Owner Author

@hannesm hannesm commented Feb 6, 2016

There is no longer any need for force_redraw and thus no hex characters in the lower left corner. The resulting binary is self-sufficient (no need for camomile data files) and roughly 13MB in size (on an amd64). After decrypting a message, the resulting data is validated to be UTF-8.

New features include that suggestions for commands are now rendered in lightgray (type / to see an example); the configuration dialog has been rewritten and does not exit the program anymore if invalid data is entered.

hannesm added 16 commits Feb 6, 2016
fix
…e (and how to proceed)
@hannesm
Copy link
Owner Author

@hannesm hannesm commented Feb 7, 2016

I think this is good now... will see what reports I get in ;)

hannesm added a commit that referenced this pull request Feb 7, 2016
Refactor data flow and terminal library
@hannesm hannesm merged commit f827b1f into master Feb 7, 2016
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
@hannesm hannesm deleted the notty branch Feb 7, 2016
@pqwy

This comment has been minimized.

Copy link

@pqwy pqwy commented on cli/cli_client.ml in 4dc0139 Feb 7, 2016

The case split is redundant.

This comment has been minimized.

Copy link
Owner Author

@hannesm hannesm replied Feb 7, 2016

thx, I moved the whole newline processing to split_on_nl in 26169d1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.