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

flush buffered keystrokes after external viewer #665

Merged
merged 1 commit into from
Jul 14, 2017

Conversation

rolandwalker
Copy link
Contributor

@rolandwalker rolandwalker commented Jul 14, 2017

by reopening the TTY seeking to SEEK_END. Otherwise keystrokes pressed prior to Enter hang around and can get read later.

To reproduce:

  • :bind generic o !ls
  • o # shows listing
  • qEnter # leaves q unflushed
  • o # does not show listing; returns immediately due to unflushed character

@jonas
Copy link
Owner

jonas commented Jul 14, 2017

Looks like this will fix #232

@jonas
Copy link
Owner

jonas commented Jul 14, 2017

I misread, but hope to have found a fix for #232 because of this ticket.

src/display.c Outdated
@@ -64,6 +64,9 @@ open_external_viewer(const char *argv[], const char *dir, bool silent, bool conf
fprintf(stderr, "%s", notice);
fprintf(stderr, "Press Enter to continue");
getc(opt_tty);
opt_tty = freopen(NULL, "r", opt_tty);
Copy link
Owner

@jonas jonas Jul 14, 2017

Choose a reason for hiding this comment

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

If I read freopen(3) correctly, the mode should be "w+" to match how it is initially opened in pager mode:

out_tty = no_display ? fopen("/dev/null", "w+") : opt_tty;

@jonas
Copy link
Owner

jonas commented Jul 14, 2017

Looks good to me, but I am not sure if this means that the terminal also needs to be reopened somehow. From my understanding of how newterm works the FILE * passed there is used for setting up the terminfo DB etc.

@jonas
Copy link
Owner

jonas commented Jul 14, 2017

Also somewhat related to the suspending cleanups in #657

@rolandwalker rolandwalker force-pushed the flush-after-external-viewer branch from c5f9b14 to cb17007 Compare July 14, 2017 22:07
@rolandwalker
Copy link
Contributor Author

Seeking to SEEK_END also solves the test case and is more conservative.

I missed #657 and haven't tested the interactions.

@jonas
Copy link
Owner

jonas commented Jul 14, 2017

Yes, that looks safer.

@jonas jonas merged commit 267a2ea into jonas:master Jul 14, 2017
@rolandwalker rolandwalker deleted the flush-after-external-viewer branch July 14, 2017 22:33
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.

2 participants