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

Reject piped input (/dev/stdin) for BedToIntervalList #1918

Merged
merged 3 commits into from
Mar 12, 2024

Conversation

kockan
Copy link
Contributor

@kockan kockan commented Nov 14, 2023

As the title states, reject piped input (/dev/stdin) for BedToIntervalList since it cannot process such input.

@kockan
Copy link
Contributor Author

kockan commented Nov 14, 2023

For #1890

Copy link
Contributor

@droazen droazen left a comment

Choose a reason for hiding this comment

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

One comment/request

src/main/java/picard/util/BedToIntervalList.java Outdated Show resolved Hide resolved
@kockan kockan requested a review from droazen February 15, 2024 20:54
@@ -114,8 +114,13 @@ public class BedToIntervalList extends CommandLineProgram {
@Override
protected int doWork() {
IOUtil.assertFileIsReadable(INPUT);
if(INPUT.getPath().equals("/dev/stdin")) {
throw new IllegalArgumentException("BedToIntervalList cannot read from /dev/stdin.");
Copy link
Contributor

@cmnbroad cmnbroad Mar 12, 2024

Choose a reason for hiding this comment

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

This is a bit of a nit, but this error message could be interpreted to mean that reading from the input is acceptable, but just failing for some reason - I would suggest explicitly stating that reading from stdin is not supported:

Suggested change
throw new IllegalArgumentException("BedToIntervalList cannot read from /dev/stdin.");
throw new IllegalArgumentException("BedToIntervalList does not support reading from standard input - a file must be provided");

We should also add a test to verify this.

@kockan I'm happy to make these changes if you like - just let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cmnbroad Of course, please feel free to push to my branch

@cmnbroad cmnbroad force-pushed the can_bed2intervallist_stdin_check branch from 4a8976f to ccdbb90 Compare March 12, 2024 14:46
@droazen droazen merged commit 5b0b4c0 into master Mar 12, 2024
6 checks passed
@droazen droazen deleted the can_bed2intervallist_stdin_check branch March 12, 2024 17:08
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.

3 participants