Review Round 2

include/seqan3/argument_parser/validators.hpp

@@ -574,7 +574,7 @@ class input_file_validator : public file_validator_base
     }
 };
 
-//!\brief Mode of an output file: Determines whether an existing file is accepted for writing.
+//!\brief Mode of an output file: Determines whether an existing file can be (silently) overwritten.
 enum class output_file_open_options
 {
     //!\brief Allow to overwrite the output file
@@ -592,7 +592,7 @@ enum class output_file_open_options
  *
  * On construction, the validator can receive a list (std::vector over std::string) of valid file extensions.
  * The class acts as a functor that throws a seqan3::validation_error exception whenever a given filename's
- * extension (sts::string) is not in the given list of valid file extensions, if the file already exist, or if the
+ * extension (std::string) is not in the given list of valid file extensions, if the file already exist, or if the
  * parent path does not have the proper writer permissions.
  * In addition, the validator receives a parameter, which allows (or forbids) to overwrite an existing output file.
  *
@@ -621,49 +621,47 @@ class output_file_validator : public file_validator_base
      */
 
     //!\copydoc seqan3::input_file_validator::input_file_validator()
-    output_file_validator() = default; //!< Defaulted.
+    output_file_validator() : output_file_validator{output_file_open_options::create_new}
+    {}
+
     output_file_validator(output_file_validator const &) = default; //!< Defaulted.
     output_file_validator(output_file_validator &&) = default; //!< Defaulted.
     output_file_validator & operator=(output_file_validator const &) = default; //!< Defaulted.
     output_file_validator & operator=(output_file_validator &&) = default; //!< Defaulted.
     virtual ~output_file_validator() = default; //!< Virtual Destructor.
 
-    /*!\brief Constructs from a given collection of valid extensions.
+    /*!\brief Constructs from a given mode to allow or forbid overwriting of the file and a collection of valid
+     *        extensions.
      * \param[in] mode The property whether it is valid to overwrite the output file.
-     *
-     * \details
-     *
-     * This constructor is only available if `file_t` does not name a valid seqan3 file type that contains a
-     * static member `valid_formats`.
+     * \param[in] extensions The valid extensions to validate for. Defaults to
+     *                       seqan3::output_file_validator::default_extensions.
      */
-    explicit output_file_validator(output_file_open_options const mode) : file_validator_base{}, mode{mode}
+    explicit output_file_validator(output_file_open_options const mode,
+                                   std::vector<std::string> extensions = default_extensions())
+        : file_validator_base{}, mode{mode}
     {
-        if constexpr (!std::same_as<file_t, void>)
-            file_validator_base::extensions = detail::valid_file_extensions<typename file_t::valid_formats>();
+        file_validator_base::extensions = std::move(extensions);
     }
 
-    /*!\brief Constructs from a given collection of valid extensions.
-     * \param[in] extensions The valid extensions to validate for.
-     * \param[in] mode The property whether it is valid to overwrite the output file.
+    // Import base constructor.
+    using file_validator_base::file_validator_base;
+    //!\}
+
+    /*!\brief The default extensions of `file_t`.
+     * \returns A list of default extensions for `file_t`, will be empty if `file_t` is `void`.
      *
      * \details
      *
-     * This constructor is only available if `file_t` does not name a valid seqan3 file type that contains a
-     * static member `valid_formats`.
+     * If `file_t` does name a valid seqan3 file type that contains a static member `valid_formats` returns the
+     * extensions of that `file_t` type. Otherwise returns an empty list.
      */
-    explicit output_file_validator(output_file_open_options const mode, std::vector<std::string> extensions)
-    //!\cond
-        requires std::same_as<file_t, void>
-    //!\endcond
-        : file_validator_base{}, mode{mode}
+    static std::vector<std::string> default_extensions()
     {
-        file_validator_base::extensions = std::move(extensions);
+        if constexpr (!std::same_as<file_t, void>)
+            return detail::valid_file_extensions<typename file_t::valid_formats>();
+        return {};
     }
 
-    // Import base constructor.
-    using file_validator_base::file_validator_base;
-    //!\}
-
     // Import the base::operator()
     using file_validator_base::operator();
 
@@ -706,7 +704,7 @@ class output_file_validator : public file_validator_base
 
 private:
     //!\brief Stores the current mode of whether it is valid to overwrite the output file.
-    output_file_open_options mode = output_file_open_options::create_new;
+    output_file_open_options mode{output_file_open_options::create_new};
 };
 
 /*!\brief A validator that checks if a given path is a valid input directory.

test/snippet/argument_parser/validators_output_file.cpp

@@ -10,12 +10,12 @@ int main(int argc, const char ** argv)
     std::filesystem::path myfile{};
 
                         // Add the output_file_open_options, to allow overwriting existing files, ...
-    myparser.add_option(myfile,'f',"file","Output file containing the processed sequences.",
+    myparser.add_option(myfile, 'f', "file", "Output file containing the processed sequences.",
                         seqan3::option_spec::DEFAULT,
                         seqan3::output_file_validator{seqan3::output_file_open_options::open_or_create, {"fa","fasta"}});
 
                         // ... or avoid this and force to create a new file.
-    myparser.add_option(myfile,'g',"file2","Output file containing the processed sequences.",
+    myparser.add_option(myfile, 'g', "file2", "Output file containing the processed sequences.",
                         seqan3::option_spec::DEFAULT,
                         seqan3::output_file_validator{seqan3::output_file_open_options::create_new, {"fa","fasta"}});
     //! [validator_call]

test/unit/argument_parser/format_parse_validators_test.cpp

@@ -201,15 +201,17 @@ TEST(validator_test, input_file_ext_from_file)
 TEST(validator_test, output_file)
 {
     seqan3::test::tmp_filename tmp_name{"testbox.fasta"};
+    std::filesystem::path not_existing_path{tmp_name.get_path()};
     seqan3::test::tmp_filename tmp_name_2{"testbox_2.fasta"};
+    std::ofstream tmp_file_2(tmp_name_2.get_path());    // create file
+    std::filesystem::path existing_path{tmp_name_2.get_path()};
     seqan3::test::tmp_filename tmp_name_3{"testbox_3.fa"};
 
     std::vector formats{std::string{"fa"}, std::string{"sam"}, std::string{"fasta"}};
 
     { // single file
 
         { // file does not exist (& no formats given)
-            std::filesystem::path not_existing_path{tmp_name_2.get_path()};
             seqan3::output_file_validator my_validator{seqan3::output_file_open_options::open_or_create};
             EXPECT_NO_THROW(my_validator(not_existing_path));
             seqan3::output_file_validator my_validator2{seqan3::output_file_open_options::create_new};
@@ -219,15 +221,11 @@ TEST(validator_test, output_file)
         }
 
         { // file does exist & overwriting is prohibited
-            std::ofstream tmp_file_2(tmp_name_2.get_path());
-            std::filesystem::path existing_path{tmp_name_2.get_path()};
             seqan3::output_file_validator my_validator{seqan3::output_file_open_options::create_new, formats};
             EXPECT_THROW(my_validator(existing_path), seqan3::validation_error);
         }
 
         { // file does exist but allow to overwrite it
-            std::ofstream tmp_file_2(tmp_name_2.get_path());
-            std::filesystem::path existing_path{tmp_name_2.get_path()};
             seqan3::output_file_validator my_validator{seqan3::output_file_open_options::open_or_create, formats};
             EXPECT_NO_THROW(my_validator(existing_path));
         }
@@ -343,7 +341,7 @@ TEST(validator_test, output_file_ext_from_file)
     // Give as a template argument the seqan3 file type to get all valid extensions for this file.
     seqan3::output_file_validator<dummy_file> validator1{};
     EXPECT_EQ(validator1.get_help_page_message(), "The output file must not exist already and write permissions must "
-                                                  "be granted.");
+                                                  "be granted. Valid file extensions are: [fa, fasta, sam, bam].");
 
     seqan3::output_file_validator<dummy_file> validator2{seqan3::output_file_open_options::create_new};
     EXPECT_EQ(validator2.get_help_page_message(), "The output file must not exist already and write permissions must "