-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor #3
Conversation
TimonPost
commented
Sep 26, 2019
•
edited
Loading
edited
- book -> lib.rs
- removed /src/input/input.rs
- some namespace refactoring
- added some examples and made them compile
- ITerminalInput -> Input
- removed unsafe code
- handled tab WinApi property
src/input.rs
Outdated
#[cfg(unix)] | ||
mod unix_input; | ||
pub(crate) mod unix_input; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to follow style of what we're doing now here - drop the _unix
suffix here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 👍 means okay, but in another PR?
src/input/windows_input.rs
Outdated
@@ -41,7 +44,7 @@ const ENABLE_MOUSE_MODE: u32 = 0x0010 | 0x0080 | 0x0008; | |||
// NOTE (@imdaveho): this global var is terrible -> move it elsewhere... | |||
static mut ORIG_MODE: u32 = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we fix the static mut
in this PR as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review re-requested, but it's still there. Does it mean that it will be in another PR?
LGTM except couple of things:
|
Sorry, I figured the up-to-date code was not yet pushed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I will follow up this one with docs PR.
@@ -38,10 +42,29 @@ impl WindowsInput { | |||
|
|||
const ENABLE_MOUSE_MODE: u32 = 0x0010 | 0x0080 | 0x0008; | |||
|
|||
// NOTE (@imdaveho): this global var is terrible -> move it elsewhere... | |||
static mut ORIG_MODE: u32 = 0; | |||
lazy_static! { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this & ENABLE_MOUSE_MODE
above the pub(crate) struct WindowsInput;
line? Just to keep constants, statics, ... at the "top" (below imports & modules).