libos+apps: Make the vector argument more flexible

apps/cat.cpp

@@ -23,17 +23,13 @@ static Result<void> do_cat(StringView path)
 
 Result<int> luna_main(int argc, char** argv)
 {
-    StringView filename;
     Vector<StringView> files;
 
     os::ArgumentParser parser;
     parser.add_description("Concatenate files to standard output."_sv);
     parser.add_system_program_info("cat"_sv);
-    parser.add_positional_argument(filename, "file"_sv, "-"_sv);
-    parser.set_vector_argument(files);
-    parser.parse(argc, argv);
-
-    TRY(do_cat(filename));
+    parser.set_vector_argument(files, "files"_sv, "-"_sv);
+    TRY(parser.parse(argc, argv));
 
     for (auto file : files) TRY(do_cat(file));
 

apps/cp.cpp

@@ -5,20 +5,8 @@
 
 using os::File;
 
-Result<int> luna_main(int argc, char** argv)
+Result<void> copy_file(StringView source, StringView destination, bool verbose)
 {
-    StringView source;
-    StringView destination;
-    bool verbose { false };
-
-    os::ArgumentParser parser;
-    parser.add_description("Copy files or directories."_sv);
-    parser.add_system_program_info("cp"_sv);
-    parser.add_positional_argument(source, "source"_sv, true);
-    parser.add_positional_argument(destination, "destination"_sv, true);
-    parser.add_switch_argument(verbose, 'v', "verbose"_sv, "show more information"_sv);
-    parser.parse(argc, argv);
-
     SharedPtr<File> source_file = TRY(os::File::open(source, File::ReadOnly));
     SharedPtr<File> destination_file;
 
@@ -46,5 +34,24 @@ Result<int> luna_main(int argc, char** argv)
         destination_file->write(buf);
     }
 
+    return {};
+}
+
+Result<int> luna_main(int argc, char** argv)
+{
+    Vector<StringView> files;
+    StringView destination;
+    bool verbose { false };
+
+    os::ArgumentParser parser;
+    parser.add_description("Copy files or directories."_sv);
+    parser.add_system_program_info("cp"_sv);
+    parser.set_vector_argument(files, "files"_sv, true);
+    parser.add_positional_argument(destination, "destination"_sv, true);
+    parser.add_switch_argument(verbose, 'v', "verbose"_sv, "show more information"_sv);
+    parser.parse(argc, argv);
+
+    for (const auto& file : files) { TRY(copy_file(file, destination, verbose)); }
+
     return 0;
 }

apps/time.cpp

@@ -11,7 +11,7 @@ Result<int> luna_main(int argc, char** argv)
     os::ArgumentParser parser;
     parser.add_description("Time a command.");
     parser.add_system_program_info("time"_sv);
-    parser.set_vector_argument(command, true);
+    parser.set_vector_argument(command, "command"_sv, true, true);
     TRY(parser.parse(argc, argv));
 
     auto pid = TRY(os::Process::fork());

libos/include/os/ArgumentParser.h

@@ -84,16 +84,31 @@ namespace os
         Result<void> add_value_argument(StringView& out, char short_flag, StringView long_flag, StringView help = {});
 
         /**
-         * @brief Set the vector to append extra unregistered positional arguments to, after all
-         * registered positional arguments have been parsed. (If no vector is set, these values will instead be
-         * discarded!)
+         * @brief Register a positional argument vector (can be only done once).
          *
-         * @param out The vector of StringViews to use.
+         * @param out The vector of StringViews to store arguments in.
+         * @param name The name to give to the argument vector (in the help text).
+         * @param fallback The value to put in the vector if no values are provided.
          * @param allow_no_more_flags If set, after starting to append values into the vector, no more flags (switch and
          * value arguments) will be parsed, and will instead be treated as regular positional arguments, as if '--' had
          * been specified on the command line.
          */
-        void set_vector_argument(Vector<StringView>& out, bool allow_no_more_flags = false);
+        Result<void> set_vector_argument(Vector<StringView>& out, StringView name, StringView fallback,
+                                         bool allow_no_more_flags = false);
+
+        /**
+         * @brief Register a positional argument vector (can be only done once).
+         *
+         * @param out The vector of StringViews to store arguments in.
+         * @param name The name to give to the argument vector (in the help text).
+         * @param required Whether the user must enter at least one value for this argument. If this is false and the
+         * user does not enter a value, out will be cleared.
+         * @param allow_no_more_flags If set, after starting to append values into the vector, no more flags (switch and
+         * value arguments) will be parsed, and will instead be treated as regular positional arguments, as if '--' had
+         * been specified on the command line.
+         */
+        Result<void> set_vector_argument(Vector<StringView>& out, StringView name, bool required,
+                                         bool allow_no_more_flags = false);
 
         /**
          * @brief Parse the given command-line using this ArgumentParser's registered arguments.

libos/src/ArgumentParser.cpp

@@ -65,11 +65,28 @@ namespace os
         return m_value_args.try_append(move(arg));
     }
 
-    void ArgumentParser::set_vector_argument(Vector<StringView>& out, bool allow_no_more_flags)
+    Result<void> ArgumentParser::set_vector_argument(Vector<StringView>& out, StringView name, StringView fallback,
+                                                     bool allow_no_more_flags)
     {
+        if (m_vector_argument) return err(EINVAL);
         m_vector_argument = &out;
         m_allow_no_more_flags_after_vector_argument_start = allow_no_more_flags;
-        return;
+
+        PositionalArgument arg { nullptr, name, false, fallback };
+
+        return m_positional_args.try_append(move(arg));
+    }
+
+    Result<void> ArgumentParser::set_vector_argument(Vector<StringView>& out, StringView name, bool required,
+                                                     bool allow_no_more_flags)
+    {
+        if (m_vector_argument) return err(EINVAL);
+        m_vector_argument = &out;
+        m_allow_no_more_flags_after_vector_argument_start = allow_no_more_flags;
+
+        PositionalArgument arg { nullptr, name, required, {} };
+
+        return m_positional_args.try_append(move(arg));
     }
 
     // Change this every year!
@@ -117,6 +134,7 @@ namespace os
         bool is_parsing_value_argument = false;
 
         bool is_still_parsing_flags = true;
+        bool is_parsing_argument_vector = false;
 
         Vector<PositionalArgument> positional_args = TRY(m_positional_args.shallow_copy());
 
@@ -232,14 +250,21 @@ namespace os
                 }
             }
 
-            Option<PositionalArgument> current = positional_args.try_dequeue();
-            if (!current.has_value())
+            if (is_parsing_argument_vector)
             {
-                if (m_vector_argument)
-                {
-                    TRY(m_vector_argument->try_append(arg));
-                    if (m_allow_no_more_flags_after_vector_argument_start) is_still_parsing_flags = false;
-                }
+                TRY(m_vector_argument->try_append(arg));
+                continue;
+            }
+
+            Option<PositionalArgument> current = positional_args.try_dequeue();
+            if (!current.has_value()) continue;
+
+            if (!current->out)
+            {
+                is_parsing_argument_vector = true;
+                m_vector_argument->clear();
+                TRY(m_vector_argument->try_append(arg));
+                if (m_allow_no_more_flags_after_vector_argument_start) is_still_parsing_flags = false;
                 continue;
             }
 
@@ -253,15 +278,47 @@ namespace os
             short_usage(program_name);
         }
 
-        // Loop through all remaining positional arguments.
-        for (const auto& arg : positional_args)
+        if (is_parsing_argument_vector)
         {
-            if (arg.required)
+            // Fill the positional arguments after the vector using the vector's last elements.
+            usize i = 1;
+            usize remaining_args = positional_args.size();
+            if (remaining_args < m_vector_argument->size()) i = m_vector_argument->size() - remaining_args;
+            for (const auto& arg : positional_args)
             {
-                os::eprintln("%s: required argument '%s' not provided", program_name.chars(), arg.name.chars());
-                short_usage(program_name);
+                if (i >= m_vector_argument->size())
+                {
+                    if (arg.required)
+                    {
+                        os::eprintln("%s: required argument '%s' not provided", program_name.chars(), arg.name.chars());
+                        short_usage(program_name);
+                    }
+                    else { *arg.out = arg.fallback; }
+                }
+                else { *arg.out = m_vector_argument->remove_at(i); }
+            }
+            check(i >= m_vector_argument->size());
+        }
+        else
+        {
+            // Loop through all remaining positional arguments.
+            for (const auto& arg : positional_args)
+            {
+                if (arg.required)
+                {
+                    os::eprintln("%s: required argument '%s' not provided", program_name.chars(), arg.name.chars());
+                    short_usage(program_name);
+                }
+                else
+                {
+                    if (arg.out) *arg.out = arg.fallback;
+                    else
+                    {
+                        m_vector_argument->clear();
+                        TRY(m_vector_argument->try_append(arg.fallback));
+                    }
+                }
             }
-            else { *arg.out = arg.fallback; }
         }
 
         return {};
@@ -276,10 +333,9 @@ namespace os
         {
             if (arg.required) { TRY(sb.format(" %s", arg.name.chars())); }
             else { TRY(sb.format(" [%s]", arg.name.chars())); }
+            if (!arg.out) sb.add("..."_sv);
         }
 
-        if (m_vector_argument) TRY(sb.format("%s..."_sv, m_positional_args.size() ? "" : " "));
-
         TRY(sb.add('\n'));
 
         auto usage_line = TRY(sb.string());