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

[FEATURE] file validators should handle stdin and stdout properly #124

Open
2 tasks done
Tracked by #97
h-2 opened this issue Sep 27, 2022 · 10 comments
Open
2 tasks done
Tracked by #97

[FEATURE] file validators should handle stdin and stdout properly #124

h-2 opened this issue Sep 27, 2022 · 10 comments

Comments

@h-2
Copy link
Member

h-2 commented Sep 27, 2022

Does this problem persist on the current main?

  • I have verified the issue on the current main

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Passing "-" to an input_file_validator fails with

what():  Validation failed for positional option 1: The file "-" does not exist!`

Passing "/dev/stdin" also fails:

what():  Validation failed for positional option 1: Expected a regular file "/dev/stdin"!

Expected Behavior

Both of these values should work.

Both of them should also be exempt from extension-checks (or there should be another validator called input_file_or_stdin_validator or maybe even just input_validator because it covers all possible inputs?).

P.S.: independent of "/dev/stdout", I think it is too restrictive to only allow regular files. Being able to open FIFOs is useful, too.

Steps To Reproduce

try it :)

Environment

- Operating system: Ubuntu 22.04
- Sharg version: 89bb8e71172ceea20b95beed5e071fe62b13d91c
- Compiler: gcc-11.3

Anything else?

I like the config being switched to designated initialisers 👍🏻

Suggestion: the documentation would be easier to read if validators and exceptions were in their own folder, because there would be fewer items cluttering the top-level of the documentation. They could even have their own namespace (sharg::validator::input_file ...).

@h-2 h-2 added the bug Something isn't working label Sep 27, 2022
@smehringer
Copy link
Member

Hi @h-2,
these are good points! We'll discuss them soon.

@eseiler
Copy link
Member

eseiler commented Sep 28, 2022

Just dumping an idea: Maybe we could do something like
sharg::input_validator{sharg::file_mode::file_only} and sharg::input_validator{..::all}.similar to the output_file_validator and with sensible names and default. Then we should change the output validator too.

I agree with the documentation suggestion of adding some more folders.

@smehringer
Copy link
Member

smehringer commented Sep 29, 2022

Core Meeting 29.09.2022

Tasks after decisions:

  • drop the restriction that input_file_validator must be a is_regular_file to also allow FIFO, sockets etc.
  • input_file_validator does not check extensions for - and /dev/stdin
  • output_file_validator does not check extensions for - and /dev/stdout

This is not API relevant because file validators are still experimental.

@smehringer smehringer changed the title Poor handling of stdin [FEATURE] file validators should handle stdin and stdout properly Sep 29, 2022
@smehringer smehringer added feature/proposal and removed bug Something isn't working labels Sep 29, 2022
@smehringer
Copy link
Member

@h-2 In your specific use case, Do you want to use stdin together wit seqan3 files? Because how do you determine the format, if previously, when giving a normal filename, automatic format detection based on extension is happening?

Do you know why we do not have automated format detection?
> -> FASTA
@ -> FASTQ
Sam / BAM has a magic string in the beginning

@h-2
Copy link
Member Author

h-2 commented Nov 9, 2022

I am not using it with SeqAn3 but with BioC++ I/O where the format can be selected manually (and will likely offer magic byte detection as well).

@smehringer
Copy link
Member

Thanks for the quick reply! Does BIO not have file extension detection?

@h-2
Copy link
Member Author

h-2 commented Nov 11, 2022

Thanks for the quick reply! Does BIO not have file extension detection?

It does... but it also allows specifying the format manually. To me this unrelated to argument parsing.

@eseiler
Copy link
Member

eseiler commented Nov 11, 2022

So, this means you want to use - as input with an input_file_validator, but not with format validation?

I.e., it would be good enough for input_file_validator{} to work, but input_file_validator{{"fa", "fasta"}} not?

@h-2
Copy link
Member Author

h-2 commented Nov 11, 2022

So, this means you want to use - as input with an input_file_validator, but not with format validation?

What I meant was, that the detection in the reader is independent of the argument parsing.

I.e., it would be good enough for input_file_validator{} to work, but input_file_validator{{"fa", "fasta"}} not?

I would hope that this works. I think - should just be ignored when verifying extensions, i.e. it should just be successfull.

@h-2
Copy link
Member Author

h-2 commented Jan 10, 2023

Any news on this?

I would actually recommend just creating additional validators. It's very straightforward:

class input_file_or_stdin_validator : public sharg::input_file_validator
{
private:
    using base_t = sharg::input_file_validator;
public:

    using base_t::base_t;

    virtual void operator()(std::filesystem::path const & file) const override
    {
        if (file != "-" && file != "/dev/stdin")
            sharg::input_file_validator::operator()(file);
    }
};

class output_file_or_stdout_validator : public sharg::output_file_validator
{
private:
    using base_t = sharg::output_file_validator;
public:

    using base_t::base_t;

    virtual void operator()(std::filesystem::path const & file) const override
    {
        if (file != "-" && file != "/dev/stdout")
            sharg::output_file_validator::operator()(file);
    }
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants