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

[open-] fix open-file for - in cmdlogs #2582 #2584

Merged
merged 3 commits into from
Nov 9, 2024

Conversation

midichef
Copy link
Contributor

@midichef midichef commented Oct 24, 2024

Closes #2582 and #2583, which were bugs I introduced in #2442. The trouble is that the wrong file is tested for being a terminal. When command | vd is run in a terminal, duptty() eventually turns sys.stdin into a terminal. The right file to test is stdinSource.fptext.

@anjakefala How can I add some tests that run commands in a shell? I'd like to add tests that pipe to vd, like:
cat sample.tsv | vd -
cat sample.tsv | vd -p -
cat sample.tsv | vd -p open-file.vdj

They'll be good for testing the line of code added in this commit, when I submit a patch to make piped data be binary, instead of text.

@midichef
Copy link
Contributor Author

vd -p - would lead to a nonresponsive terminal. Similar to the way that vd - used to, before 49a1238. So I've patched that as well.

Copy link
Owner

@saulpw saulpw left a comment

Choose a reason for hiding this comment

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

Thanks for owning this.

@anjakefala anjakefala marked this pull request as ready for review November 9, 2024 03:46
@anjakefala anjakefala merged commit 2af438e into saulpw:develop Nov 9, 2024
14 checks passed
@midichef midichef deleted the stdin_pipe_tty branch December 1, 2024 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replaying a cmdlog where input is read from stdin fails
3 participants