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

reset-console #1496

Merged
merged 3 commits into from Jun 26, 2017
Merged

reset-console #1496

merged 3 commits into from Jun 26, 2017

Conversation

rouzier
Copy link
Contributor

@rouzier rouzier commented Jun 20, 2017

This pull request resets stdin to be read from the current console on unix based system.

To avoid the vim error 'Vim: Warning: Input is not from a terminal'

When reading in the pull request message from stdin and editing it afterwards.

For example the following command will no longer causing issues for a terminal (Which is the command I used to create this pull request)

(git rev-parse --abbrev-ref HEAD 2>/dev/null;(git diff --no-color -M --patch-with-stat  github/master...HEAD | perl -pi -e's/^/#/') )  | hub pull-request -b github:master -F - --edit

Fixes #1481

@mislav
Copy link
Owner

mislav commented Jun 21, 2017

Interesting approach; thank you for trying to solve this!

I'm wondering whether there might be a potentially less intrusive way, i.e. one that doesn't force us to reset anything for the curent hub process.

The root of the problem is that the child vim process inherits the stdin from its parent hub process, which is undesireable in this case. Can't we simply change the stdin for the child vim process to be connected to the original terminal? Something like:

editCmd.Stdin = ...
editCmd.Spawn()

@rouzier
Copy link
Contributor Author

rouzier commented Jun 21, 2017

Let me try that approach.

@mislav
Copy link
Owner

mislav commented Jun 23, 2017

This looks great. I wanted to ask what would happen if tty was unavailable in the first place, for example in environments was called from a cron job or within a process that doesn't have a terminal, but opening vim wouldn't work or make sense in that case anyway, so I think we're safe like this.

Have you tried that this fixes the problem for you?

@rouzier
Copy link
Contributor Author

rouzier commented Jun 23, 2017 via email

@mislav mislav merged commit 414f47d into mislav:master Jun 26, 2017
@mislav
Copy link
Owner

mislav commented Jun 26, 2017

Thank you for the hard work!

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.

None yet

2 participants