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.

Sign up for GitHub

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

Jump to bottom

use read syscall directly to get character #931

Merged
merged 1 commit into from
May 24, 2017
Merged

use read syscall directly to get character #931

merged 1 commit into from
May 24, 2017

Conversation

tw4452852
Copy link
Contributor

Fix issue #910 .

@junegunn
Copy link
Owner

Thanks, I'd prefer to have the comment as the commit message, can you move it?

@tw4452852
Copy link
Contributor Author

Done.

@junegunn
Copy link
Owner

Oh well, it doesn't compile for Windows.

> GOOS=windows make release
cd /Users/jg/.vim/plugged/fzf/gopath/src/github.com/junegunn/fzf/src && go get -tags ""
# github.com/junegunn/fzf/src/tui
tui/light.go:256: cannot use fd (type int) as type syscall.Handle in argument to syscall.Read
make: *** [deps] Error 2

We might need to move getch code to util_unix and util_windows.

@tw4452852
use read syscall directly to get character
942f07d 
Due to go std lib uses poller for os.File introducing in this commit:
golang/go@c05b06a
There are two changes to watch out:
1. os.File.Fd will always return a blocking fd except on bsd.
2. os.File.Read won't return EAGAIN error for nonblocking fd.

So
For 1, we just get tty's fd in advance and then set its block mode.
For 2, we use read syscall directly to get what we wanted error(EAGAIN).

Fix issue junegunn#910.

Signed-off-by: Tw <tw19881113@gmail.com>
@tw4452852
Copy link
Contributor Author

Hmm, I just work out syscall.Read for different platforms instead.

BTW, light.go isn't for windows apparently, because it gets input from /dev/tty which doesn't exist on windows. For now, we disable --height option on windows, so LightRenderer won't be used on windows. I think we should add +build !windows to light.go.

@junegunn
Copy link
Owner

junegunn commented May 24, 2017 •
edited
Loading

Thanks, you're right, but I think it's good enough for now. I'll merge your commit to devel branch first.

@junegunn junegunn changed the base branch from master to devel May 24, 2017 16:36
@junegunn junegunn merged commit ab182e2 into junegunn:devel May 24, 2017
@tw4452852 tw4452852 deleted the 910 branch May 25, 2017 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tw4452852 @junegunn