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

Fix msys detection with new winapi. #28

Merged
merged 1 commit into from
Jul 22, 2018
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jul 14, 2018

Ever since winapi version 0.3.5, tty detection for msys terminals has been
broken. The definition for FILE_NAME_INFO struct changed in
retep998/winapi-rs#607, causing the first character from the filename to be cut
off.

src/lib.rs Outdated
@@ -26,6 +26,7 @@ extern crate termion;

#[cfg(windows)]
use winapi::shared::minwindef::DWORD;
use winapi::shared::ntdef::WCHAR;
Copy link

Choose a reason for hiding this comment

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

Missing #[cfg(windows)] for winapi::shared::ntdef::WCHAR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, thx!

Ever since winapi version 0.3.5, tty detection for msys terminals has been
broken.  The definition for FILE_NAME_INFO struct changed in
retep998/winapi-rs#607, causing the first character from the filename to be cut
off.
@BurntSushi
Copy link
Contributor

Nice fix! I think I buy it. @retep998 Was this sort of breakage intentional? Or was the way we were using FILE_NAME_INFO previously just flat out wrong?

@BurntSushi
Copy link
Contributor

Also, does this fix work with winapi >=0.3.0, <0.3.5?

@retep998
Copy link

retep998 commented Jul 15, 2018

That sort of breakage was not intentional, but it's also the sort of breakage that should not affect anyone because people should not be assuming that the offset of the unsized field is the same as the struct's size. The new code in this PR is definitely much better. Yes, it works with all winapi versions.

@ehuss
Copy link
Contributor Author

ehuss commented Jul 15, 2018

Also, does this fix work with winapi >=0.3.0, <0.3.5?

I tested with winapi 0.3.0 on a variety of windows shells, and they all seem to behave correctly.

BurntSushi added a commit to BurntSushi/ripgrep that referenced this pull request Jul 22, 2018
winapi 0.3.5 changed how it represents some of its structs, which caused
a bug to surface in atty that prevents tty detection on Windows. atty
has an open PR to fix this: softprops/atty#28

Until a new release of atty, we pin winapi to a version that works.
@softprops softprops merged commit e5e3fc7 into softprops:master Jul 22, 2018
@softprops
Copy link
Owner

Thank you folks!

@softprops
Copy link
Owner

@retep998 @BurntSushi I tagged and published a new crate version 0.2.11

@BurntSushi
Copy link
Contributor

@softprops Awesome, thank you!

kennytm added a commit to kennytm/rust that referenced this pull request Jul 24, 2018
Fix color detection for Windows msys terminals.

Updates atty to pull in the fix softprops/atty#28.  Closes rust-lang/cargo#5724.
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.

5 participants