search

conradkleinespel / rpassword

Cross platform Rust library to read a password in the terminal (Linux, BSD, OSX, Windows, WASM).
Apache License 2.0
244 stars 38 forks source link

Windows support. #2

Closed retep998 closed 9 years ago

retep998 commented 9 years ago

Fixes #1

conradkleinespel commented 9 years ago

@retep998 Thanks a lot for taking the time to contribute :+1: I'm reviewing your PR right now and will comment again when I'm done.

retep998 commented 9 years ago

Comments have been added and unsafe has been localized to where needed.

conradkleinespel commented 9 years ago

@retep998 Awesome :+1: Thanks for the hard work you put in. I don't have access to OSX or Windows right now. But I'll test this ASAP and comment again then.

conradkleinespel commented 9 years ago

@retep998 Just a quick update: I haven't had the time yet to review this on OSX/Windows. I will soon though.

@dcuddeback @retep998 I think the dependencies listed in Cargo.toml are getting a little messy indeed. Until a better solution is available in Cargo, I would suggest accepting this "messy" Cargo.toml so that it works until we have a better solution available. Later, if Cargo allows an easier way to do that or if @dcuddeback 's library compile on Windows and @retep998 's libraries compile on UNIX, we can just get rid of the "messy" declarations altogether and put some more #[cfg(...)] in the code.

dcuddeback commented 9 years ago

@conradkleinespel The "messy" dependencies is known to the Cargo team. It's mentioned as one of the drawbacks in the RFC: "As can be seen in the example repository, platform dependencies are quite verbose and are difficult to work with when you actually want a negation instead of a positive platform to include." (https://github.com/rust-lang/rfcs/blob/master/text/0403-cargo-build-command.md#drawbacks) I think better syntax will come in the future, as they mentioned supporting wildcards in the future when approving the RFC: https://github.com/rust-lang/meeting-minutes/blob/master/weekly-meetings/2014-10-30.md#cargo-rfc.

I don't know much about Windows development. I don't think it supports the termios API. If it does, I'd welcome adding Windows support to the termios crate.

retep998 commented 9 years ago

@dcuddeback The Windows console API is completely different to the posix model. While someone could write a higher level API that can abstract across the two models, a lower level library such as your termios crate will never be able to work on Windows.

dcuddeback commented 9 years ago

@retep998 That's what I assumed was the case. Thanks for confirming.

conradkleinespel commented 9 years ago

@retep998 Quick update: I haven't had any time for OSS last week. But I haven't forgotten about this PR and will test it soon.