From 2f08e0f5b09bedf15ff01a5a708323cab9da7e6f Mon Sep 17 00:00:00 2001 From: apio Date: Sat, 17 Jun 2023 20:58:54 +0200 Subject: [PATCH] libos: Make long value arguments use '=' and make value arguments' values always required --- apps/date.cpp | 3 +- apps/ls.cpp | 2 +- apps/mkdir.cpp | 2 +- apps/mount.cpp | 4 +-- apps/sh.cpp | 2 +- apps/sysfuzz.cpp | 4 +-- libluna/include/luna/StringView.h | 3 ++ libluna/src/StringView.cpp | 30 ++++++++++++++-- libos/include/os/ArgumentParser.h | 26 +------------- libos/src/ArgumentParser.cpp | 60 +++++++++++++------------------ 10 files changed, 64 insertions(+), 72 deletions(-) diff --git a/apps/date.cpp b/apps/date.cpp index 9f3d21b9..8a63493c 100644 --- a/apps/date.cpp +++ b/apps/date.cpp @@ -11,8 +11,7 @@ Result luna_main(int argc, char** argv) os::ArgumentParser parser; parser.add_description("Display the current (or another) date and time."_sv); parser.add_system_program_info("date"_sv); - parser.add_value_argument(date, 'd', "date"_sv, true, - "the UNIX timestamp to display instead of the current time"_sv); + parser.add_value_argument(date, 'd', "date"_sv, "the UNIX timestamp to display instead of the current time"_sv); parser.parse(argc, argv); time_t now; diff --git a/apps/ls.cpp b/apps/ls.cpp index 4890ce1a..b5d4fe58 100644 --- a/apps/ls.cpp +++ b/apps/ls.cpp @@ -96,7 +96,7 @@ Result luna_main(int argc, char** argv) "follow symbolic links listed as arguments"_sv); parser.add_switch_argument(one_per_line, '1', ""_sv, "list one file per line"_sv); parser.add_switch_argument(list_directories, 'd', "directory"_sv, "list directories instead of their contents"_sv); - parser.add_value_argument(sort_type, ' ', "sort"_sv, true, "sort by name, size or time"_sv); + parser.add_value_argument(sort_type, ' ', "sort"_sv, "sort by name, size or time"_sv); parser.add_switch_argument(reverse_sort, 'r', "reverse"_sv, "reverse order while sorting"_sv); parser.parse(argc, argv); diff --git a/apps/mkdir.cpp b/apps/mkdir.cpp index d2562a40..8744bfc7 100644 --- a/apps/mkdir.cpp +++ b/apps/mkdir.cpp @@ -41,7 +41,7 @@ Result luna_main(int argc, char** argv) parser.add_description("Create directories."_sv); parser.add_system_program_info("mkdir"_sv); parser.add_positional_argument(path, "path"_sv, true); - parser.add_value_argument(mode_string, 'm', "mode"_sv, true, "set the mode for the newly created directory"); + parser.add_value_argument(mode_string, 'm', "mode"_sv, "set the mode for the newly created directory"); parser.add_switch_argument(recursive, 'p', "parents"_sv, "if parent directories do not exist, create them as well"_sv); parser.parse(argc, argv); diff --git a/apps/mount.cpp b/apps/mount.cpp index 5a18132c..b5762219 100644 --- a/apps/mount.cpp +++ b/apps/mount.cpp @@ -5,13 +5,13 @@ Result luna_main(int argc, char** argv) { StringView target; - StringView fstype; + StringView fstype { "auto" }; os::ArgumentParser parser; parser.add_description("Mount a file system."); parser.add_system_program_info("mount"_sv); parser.add_positional_argument(target, "mountpoint"_sv, true); - parser.add_value_argument(fstype, 't', "type"_sv, "auto"_sv, "the file system type to use"); + parser.add_value_argument(fstype, 't', "type"_sv, "the file system type to use"); parser.parse(argc, argv); if (mount(target.chars(), fstype.chars()) < 0) diff --git a/apps/sh.cpp b/apps/sh.cpp index 2d55e7a2..7f247eb9 100644 --- a/apps/sh.cpp +++ b/apps/sh.cpp @@ -48,7 +48,7 @@ Result luna_main(int argc, char** argv) parser.add_description("The Luna system's command shell."_sv); parser.add_system_program_info("sh"_sv); parser.add_positional_argument(path, "path"_sv, "-"_sv); - parser.add_value_argument(command, 'c', "command"_sv, true, "execute a single command and then exit"_sv); + parser.add_value_argument(command, 'c', "command"_sv, "execute a single command and then exit"_sv); parser.parse(argc, argv); if (!command.is_empty()) TRY(execute_command(command)); diff --git a/apps/sysfuzz.cpp b/apps/sysfuzz.cpp index 0eb3664f..424729ca 100644 --- a/apps/sysfuzz.cpp +++ b/apps/sysfuzz.cpp @@ -40,8 +40,8 @@ int main(int argc, char** argv) os::ArgumentParser parser; parser.add_description("System call fuzzer (invokes system calls with random arguments to test system stability)"); parser.add_system_program_info("sysfuzz"_sv); - parser.add_value_argument(times_sv, 't', "times"_sv, true, "the number of syscalls to invoke"_sv); - parser.add_value_argument(interval_sv, 'i', "interval"_sv, true, + parser.add_value_argument(times_sv, 't', "times"_sv, "the number of syscalls to invoke"_sv); + parser.add_value_argument(interval_sv, 'i', "interval"_sv, "the interval between system calls (in milliseconds)"_sv); parser.parse(argc, argv); diff --git a/libluna/include/luna/StringView.h b/libluna/include/luna/StringView.h index a1000b55..cc51e5c9 100644 --- a/libluna/include/luna/StringView.h +++ b/libluna/include/luna/StringView.h @@ -42,6 +42,9 @@ class StringView Result> split(StringView delim) const; Result> split_once(char delim) const; + Result> split_view(char delim) const; + + bool contains(char v) const; static StringView from_fixed_size_cstring(const char* string, usize max); diff --git a/libluna/src/StringView.cpp b/libluna/src/StringView.cpp index f3654df8..dad5eaa8 100644 --- a/libluna/src/StringView.cpp +++ b/libluna/src/StringView.cpp @@ -50,12 +50,12 @@ const char& StringView::operator[](usize index) const bool StringView::operator==(const char* other) const { - return !strcmp(m_string, other); + return !strncmp(m_string, other, m_length); } bool StringView::operator==(StringView other) const { - return !strcmp(m_string, other.m_string); + return !strncmp(m_string, other.m_string, m_length); } Result> StringView::split(StringView delim) const @@ -110,6 +110,32 @@ Result> StringView::split_once(char delim) const return result; } +Result> StringView::split_view(char delim) const +{ + Vector result; + + char* middle = strchr(m_string, delim); + if (!middle) + { + TRY(result.try_append(*this)); + return result; + } + + // begin will not contain a null terminator. + auto begin = StringView::from_fixed_size_cstring(m_string, middle - m_string); + auto end = StringView { middle + 1 }; + + TRY(result.try_append(begin)); + TRY(result.try_append(end)); + + return result; +} + +bool StringView::contains(char v) const +{ + return strchr(m_string, v); +} + Result StringView::to_uint() const { const char* endptr = nullptr; diff --git a/libos/include/os/ArgumentParser.h b/libos/include/os/ArgumentParser.h index 223f29c0..5d6189c7 100644 --- a/libos/include/os/ArgumentParser.h +++ b/libos/include/os/ArgumentParser.h @@ -78,32 +78,10 @@ namespace os * @param long_flag The long flag to use for this argument, excluding the '--' prefix: an option '--example' * would be passed as 'example'. Can be a string of any length. If you do not wish this argument to have a long * flag, use an empty string: "". - * @param fallback The value to use if the user uses the argument but does not provide a value. If, however, the - * user does not specify this argument at all, out WILL NOT BE MODIFIED! * @param help The help text to show for this argument (optional). * @return Result Whether the operation succeeded. */ - Result add_value_argument(StringView& out, char short_flag, StringView long_flag, StringView fallback, - StringView help = {}); - - /** - * @brief Register a new value argument. - * - * @param out The variable where the argument's value will be stored. - * @param short_flag The short flag to use for this argument, excluding the '-' prefix: an option '-c' would be - * passed as 'c'. Can only be a single ASCII character. If you do not wish this argument to have a short flag, - * use the space character: ' '. - * @param long_flag The long flag to use for this argument, excluding the '--' prefix: an option '--example' - * would be passed as 'example'. Can be a string of any length. If you do not wish this argument to have a long - * flag, use an empty string: "". - * @param value_required Whether the user is required to pass a value when using this argument. If this is - * false and the user does not enter a value along with the argument, out will be set to an empty string. If, - * however, the user does not specify this argument at all, out WILL NOT BE MODIFIED! - * @param help The help text to show for this argument (optional). - * @return Result Whether the operation succeeded. - */ - Result add_value_argument(StringView& out, char short_flag, StringView long_flag, bool value_required, - StringView help = {}); + Result 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 @@ -192,8 +170,6 @@ namespace os StringView* out; char short_flag; StringView long_flag; - bool required; - StringView fallback; StringView help; }; diff --git a/libos/src/ArgumentParser.cpp b/libos/src/ArgumentParser.cpp index bca5e40a..8dbdecf9 100644 --- a/libos/src/ArgumentParser.cpp +++ b/libos/src/ArgumentParser.cpp @@ -8,6 +8,7 @@ */ #include +#include #include #include #include @@ -52,22 +53,9 @@ namespace os } Result ArgumentParser::add_value_argument(StringView& out, char short_flag, StringView long_flag, - bool value_required, StringView help) + StringView help) { - ValueArgument arg { &out, short_flag, long_flag, value_required, {}, help }; - - if (short_flag == 'h') m_add_short_help_flag = false; - if (long_flag == "help"_sv) m_add_long_help_flag = false; - if (short_flag == 'v') m_add_short_version_flag = false; - if (long_flag == "version"_sv) m_add_long_version_flag = false; - - return m_value_args.try_append(move(arg)); - } - - Result ArgumentParser::add_value_argument(StringView& out, char short_flag, StringView long_flag, - StringView fallback, StringView help) - { - ValueArgument arg { &out, short_flag, long_flag, false, fallback, help }; + ValueArgument arg { &out, short_flag, long_flag, help }; if (short_flag == 'h') m_add_short_help_flag = false; if (long_flag == "help"_sv) m_add_long_help_flag = false; @@ -130,6 +118,8 @@ namespace os bool is_still_parsing_flags = true; + Vector positional_args = TRY(m_positional_args.deep_copy()); + for (int i = 1; i < argc; i++) { StringView arg = argv[i]; @@ -168,14 +158,19 @@ namespace os } } - for (const auto& current : m_value_args) + if (flag.contains('=')) { - if (current.long_flag == flag) + auto v = TRY(flag.split_view('=')); + StringView actual_flag = v[0]; + StringView data = v[1]; + for (const auto& current : m_value_args) { - current_value_argument = current; - is_parsing_value_argument = true; - found = true; - break; + if (current.long_flag == actual_flag) + { + *current.out = data; + found = true; + break; + } } } @@ -237,7 +232,7 @@ namespace os } } - Option current = m_positional_args.try_dequeue(); + Option current = positional_args.try_dequeue(); if (!current.has_value()) { if (m_vector_argument) @@ -253,17 +248,13 @@ namespace os if (is_parsing_value_argument) { - if (current_value_argument->required) - { - os::eprintln("%s: option '--%s' requires an argument", program_name.chars(), - current_value_argument->long_flag.chars()); - short_usage(program_name); - } - else { *current_value_argument->out = current_value_argument->fallback; } + os::eprintln("%s: option '-%c' requires an argument", program_name.chars(), + current_value_argument->short_flag); + short_usage(program_name); } // Loop through all remaining positional arguments. - for (const auto& arg : m_positional_args) + for (const auto& arg : positional_args) { if (arg.required) { @@ -317,10 +308,7 @@ namespace os for (const auto& arg : m_value_args) { - StringView value_name; - if (arg.required) value_name = "VALUE"_sv; - else - value_name = "[VALUE]"_sv; + StringView value_name = "VALUE"_sv; int field_size = 20 - (int)(arg.long_flag.length() + 1); if (field_size < 0) field_size = 0; @@ -333,11 +321,11 @@ namespace os } else if (arg.short_flag == ' ') { - printf(" --%s %-*s %s\n", arg.long_flag.chars(), field_size, value_name.chars(), arg.help.chars()); + printf(" --%s=%-*s %s\n", arg.long_flag.chars(), field_size, value_name.chars(), arg.help.chars()); } else { - printf(" -%c, --%s %-*s %s\n", arg.short_flag, arg.long_flag.chars(), field_size, value_name.chars(), + printf(" -%c, --%s=%-*s %s\n", arg.short_flag, arg.long_flag.chars(), field_size, value_name.chars(), arg.help.chars()); } }