common: remove support for positional arguments

This is currently unused and I don't like the approach anymore
regardless. If/when we need positional arguments (probably when
implementing the upcoming river-control protocol in rivertile)
they should be handled separately from flags.

This commit also improves the CLI error reporting to always print the
usage string if invalid arguments were passed.

build.zig

@@ -116,7 +116,7 @@ pub fn build(b: *zbs.Builder) !void {
 
         rivertile.step.dependOn(&scanner.step);
         rivertile.addPackage(scanner.getPkg());
-        rivertile.addPackagePath("args", "common/args.zig");
+        rivertile.addPackagePath("flags", "common/flags.zig");
         rivertile.linkLibC();
         rivertile.linkSystemLibrary("wayland-client");
 
@@ -209,7 +209,7 @@ fn addServerDeps(exe: *zbs.LibExeObjStep, scanner: *ScanProtocolsStep) void {
     exe.addPackage(wlroots);
     exe.linkSystemLibrary("wlroots");
 
-    exe.addPackagePath("args", "common/args.zig");
+    exe.addPackagePath("flags", "common/flags.zig");
 
     // TODO: remove when zig issue #131 is implemented
     scanner.addCSource(exe);

common/args.zig

@@ -1,126 +0,0 @@
-// This file is part of river, a dynamic tiling wayland compositor.
-//
-// Copyright 2021 The River Developers
-//
-// This program is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// This program is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with this program. If not, see <https://www.gnu.org/licenses/>.
-
-const std = @import("std");
-const mem = std.mem;
-const cstr = std.cstr;
-
-const root = @import("root");
-
-pub const FlagDef = struct {
-    name: [*:0]const u8,
-    kind: enum { boolean, arg },
-};
-
-pub fn Args(comptime num_positionals: comptime_int, comptime flag_defs: []const FlagDef) type {
-    return struct {
-        const Self = @This();
-
-        positionals: [num_positionals][*:0]const u8,
-        flags: [flag_defs.len]struct {
-            name: [*:0]const u8,
-            value: union {
-                boolean: bool,
-                arg: ?[*:0]const u8,
-            },
-        },
-
-        pub fn parse(argv: [][*:0]const u8) Self {
-            var ret: Self = undefined;
-
-            // Init all flags in the flags array to false/null
-            inline for (flag_defs) |flag_def, flag_idx| {
-                switch (flag_def.kind) {
-                    .boolean => ret.flags[flag_idx] = .{
-                        .name = flag_def.name,
-                        .value = .{ .boolean = false },
-                    },
-                    .arg => ret.flags[flag_idx] = .{
-                        .name = flag_def.name,
-                        .value = .{ .arg = null },
-                    },
-                }
-            }
-
-            // Parse the argv in to the positionals and flags arrays
-            var arg_idx: usize = 0;
-            var positional_idx: usize = 0;
-            outer: while (arg_idx < argv.len) : (arg_idx += 1) {
-                var should_continue = false;
-                inline for (flag_defs) |flag_def, flag_idx| {
-                    if (cstr.cmp(flag_def.name, argv[arg_idx]) == 0) {
-                        switch (flag_def.kind) {
-                            .boolean => ret.flags[flag_idx].value.boolean = true,
-                            .arg => {
-                                arg_idx += 1;
-                                ret.flags[flag_idx].value.arg = if (arg_idx < argv.len)
-                                    argv[arg_idx]
-                                else
-                                    root.fatal("flag '" ++ flag_def.name ++
-                                        "' requires an argument but none was provided!", .{});
-                            },
-                        }
-                        // TODO: this variable exists as a workaround for the fact that
-                        // using continue :outer here crashes the stage1 compiler.
-                        should_continue = true;
-                    }
-                }
-                if (should_continue) continue;
-
-                if (positional_idx == num_positionals) {
-                    root.fatal(
-                        "{} positional arguments expected but more were provided!",
-                        .{num_positionals},
-                    );
-                }
-
-                // This check should not be needed as this code is unreachable
-                // if num_positionals is 0. Howevere the stage1 zig compiler does
-                // not seem to be smart enough to realize this.
-                if (num_positionals > 0) {
-                    ret.positionals[positional_idx] = argv[arg_idx];
-                } else {
-                    unreachable;
-                }
-                positional_idx += 1;
-            }
-
-            if (positional_idx < num_positionals) {
-                root.fatal(
-                    "{} positional arguments expected but only {} were provided!",
-                    .{ num_positionals, positional_idx },
-                );
-            }
-
-            return ret;
-        }
-
-        pub fn boolFlag(self: Self, flag_name: [*:0]const u8) bool {
-            for (self.flags) |flag| {
-                if (cstr.cmp(flag.name, flag_name) == 0) return flag.value.boolean;
-            }
-            unreachable; // Invalid flag_name
-        }
-
-        pub fn argFlag(self: Self, flag_name: [*:0]const u8) ?[*:0]const u8 {
-            for (self.flags) |flag| {
-                if (cstr.cmp(flag.name, flag_name) == 0) return flag.value.arg;
-            }
-            unreachable; // Invalid flag_name
-        }
-    };
-}

common/flags.zig

@@ -0,0 +1,104 @@
+// This file is part of river, a dynamic tiling wayland compositor.
+//
+// Copyright 2021 The River Developers
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+const std = @import("std");
+const cstr = std.cstr;
+
+pub const Flag = struct {
+    name: [*:0]const u8,
+    kind: enum { boolean, arg },
+};
+
+pub fn ParseResult(comptime flags: []const Flag) type {
+    return struct {
+        const Self = @This();
+
+        const FlagData = struct {
+            name: [*:0]const u8,
+            value: union {
+                boolean: bool,
+                arg: ?[*:0]const u8,
+            },
+        };
+
+        /// Remaining args after the recognized flags
+        args: [][*:0]const u8,
+        /// Data obtained from parsed flags
+        flag_data: [flags.len]FlagData = blk: {
+            // Init all flags to false/null
+            var flag_data: [flags.len]FlagData = undefined;
+            inline for (flags) |flag, i| {
+                flag_data[i] = switch (flag.kind) {
+                    .boolean => .{
+                        .name = flag.name,
+                        .value = .{ .boolean = false },
+                    },
+                    .arg => .{
+                        .name = flag.name,
+                        .value = .{ .arg = null },
+                    },
+                };
+            }
+            break :blk flag_data;
+        },
+
+        pub fn boolFlag(self: Self, flag_name: [*:0]const u8) bool {
+            for (self.flag_data) |flag_data| {
+                if (cstr.cmp(flag_data.name, flag_name) == 0) return flag_data.value.boolean;
+            }
+            unreachable; // Invalid flag_name
+        }
+
+        pub fn argFlag(self: Self, flag_name: [*:0]const u8) ?[*:0]const u8 {
+            for (self.flag_data) |flag_data| {
+                if (cstr.cmp(flag_data.name, flag_name) == 0) return flag_data.value.arg;
+            }
+            unreachable; // Invalid flag_name
+        }
+    };
+}
+
+pub fn parse(args: [][*:0]const u8, comptime flags: []const Flag) !ParseResult(flags) {
+    var ret: ParseResult(flags) = .{ .args = undefined };
+
+    var arg_idx: usize = 0;
+    while (arg_idx < args.len) : (arg_idx += 1) {
+        var parsed_flag = false;
+        inline for (flags) |flag, flag_idx| {
+            if (cstr.cmp(flag.name, args[arg_idx]) == 0) {
+                switch (flag.kind) {
+                    .boolean => ret.flag_data[flag_idx].value.boolean = true,
+                    .arg => {
+                        arg_idx += 1;
+                        if (arg_idx == args.len) {
+                            std.log.err("flag '" ++ flag.name ++
+                                "' requires an argument but none was provided!", .{});
+                            return error.MissingFlagArgument;
+                        }
+                        ret.flag_data[flag_idx].value.arg = args[arg_idx];
+                    },
+                }
+                parsed_flag = true;
+            }
+        }
+        if (!parsed_flag) break;
+    }
+
+    ret.args = args[arg_idx..];
+
+    return ret;
+}

river/main.zig

@@ -21,8 +21,7 @@ const fs = std.fs;
 const io = std.io;
 const os = std.os;
 const wlr = @import("wlroots");
-const Args = @import("args").Args;
+const flags = @import("flags");
-const FlagDef = @import("args").FlagDef;
 
 const c = @import("c.zig");
 const util = @import("util.zig");
@@ -39,7 +38,7 @@ pub var level: std.log.Level = switch (std.builtin.mode) {
 };
 
 const usage: []const u8 =
-    \\Usage: river [options]
+    \\usage: river [options]
     \\
     \\  -h            Print this help message and exit.
     \\  -c <command>  Run `sh -c <command>` on startup.
@@ -51,28 +50,39 @@ const usage: []const u8 =
 pub fn main() anyerror!void {
     // This line is here because of https://github.com/ziglang/zig/issues/7807
     const argv: [][*:0]const u8 = os.argv;
-    const args = Args(0, &[_]FlagDef{
+    const result = flags.parse(argv[1..], &[_]flags.Flag{
         .{ .name = "-h", .kind = .boolean },
         .{ .name = "-version", .kind = .boolean },
         .{ .name = "-c", .kind = .arg },
         .{ .name = "-l", .kind = .arg },
-    }).parse(argv[1..]);
+    }) catch {
+        try io.getStdErr().writeAll(usage);
+        os.exit(1);
+    };
+    if (result.args.len != 0) {
+        std.log.err("unknown option '{s}'", .{result.args[0]});
+        try io.getStdErr().writeAll(usage);
+        os.exit(1);
+    }
 
-    if (args.boolFlag("-h")) {
+    if (result.boolFlag("-h")) {
         try io.getStdOut().writeAll(usage);
         os.exit(0);
     }
-    if (args.boolFlag("-version")) {
+    if (result.boolFlag("-version")) {
         try io.getStdOut().writeAll(@import("build_options").version);
         os.exit(0);
     }
-    if (args.argFlag("-l")) |level_str| {
+    if (result.argFlag("-l")) |level_str| {
-        const log_level = std.fmt.parseInt(u3, std.mem.span(level_str), 10) catch
+        const log_level = std.fmt.parseInt(u3, std.mem.span(level_str), 10) catch {
-            fatal("Error: invalid log level '{s}'", .{level_str});
+            std.log.err("invalid log level '{s}'", .{level_str});
+            try io.getStdErr().writeAll(usage);
+            os.exit(1);
+        };
         level = @intToEnum(std.log.Level, log_level);
     }
     const startup_command = blk: {
-        if (args.argFlag("-c")) |command| {
+        if (result.argFlag("-c")) |command| {
             break :blk try util.gpa.dupeZ(u8, std.mem.span(command));
         } else {
             break :blk try defaultInitPath();
@@ -117,11 +127,6 @@ pub fn main() anyerror!void {
     std.log.info("shutting down", .{});
 }
 
-pub fn fatal(comptime format: []const u8, args: anytype) noreturn {
-    io.getStdErr().writer().print(format ++ "\n", args) catch {};
-    os.exit(1);
-}
-
 fn defaultInitPath() !?[:0]const u8 {
     const path = blk: {
         if (os.getenv("XDG_CONFIG_HOME")) |xdg_config_home| {
@@ -141,19 +146,3 @@ fn defaultInitPath() !?[:0]const u8 {
 
     return path;
 }
-
-pub fn log(
-    comptime message_level: std.log.Level,
-    comptime scope: @TypeOf(.foobar),
-    comptime format: []const u8,
-    args: anytype,
-) void {
-    if (@enumToInt(message_level) <= @enumToInt(level)) {
-        // Don't store/log messages in release small mode to save space
-        if (std.builtin.mode != .ReleaseSmall) {
-            const stderr = io.getStdErr().writer();
-            stderr.print(@tagName(message_level) ++ ": (" ++ @tagName(scope) ++ ") " ++
-                format ++ "\n", args) catch return;
-        }
-    }
-}

rivertile/main.zig

@@ -39,17 +39,17 @@
 const std = @import("std");
 const mem = std.mem;
 const math = std.math;
+const os = std.os;
 const assert = std.debug.assert;
 
 const wayland = @import("wayland");
 const wl = wayland.client.wl;
 const river = wayland.client.river;
 
-const Args = @import("args").Args;
+const flags = @import("flags");
-const FlagDef = @import("args").FlagDef;
 
 const usage =
-    \\Usage: rivertile [options]
+    \\usage: rivertile [options]
     \\
     \\  -h, --help      Print this help message and exit.
     \\  -version        Print the version number and exit.
@@ -312,8 +312,8 @@ const Output = struct {
 
 pub fn main() !void {
     // https://github.com/ziglang/zig/issues/7807
-    const argv: [][*:0]const u8 = std.os.argv;
+    const argv: [][*:0]const u8 = os.argv;
-    const args = Args(0, &[_]FlagDef{
+    const result = flags.parse(argv[1..], &[_]flags.Flag{
         .{ .name = "-h", .kind = .boolean },
         .{ .name = "--help", .kind = .boolean },
         .{ .name = "-version", .kind = .boolean },
@@ -322,40 +322,44 @@ pub fn main() !void {
         .{ .name = "-main-location", .kind = .arg },
         .{ .name = "-main-count", .kind = .arg },
         .{ .name = "-main-ratio", .kind = .arg },
-    }).parse(argv[1..]);
+    }) catch {
+        try std.io.getStdErr().writeAll(usage);
+        os.exit(1);
+    };
+    if (result.args.len != 0) fatalPrintUsage("unknown option '{s}'", .{result.args[0]});
 
-    if (args.boolFlag("-h") or args.boolFlag("--help")) {
+    if (result.boolFlag("-h") or result.boolFlag("--help")) {
         try std.io.getStdOut().writeAll(usage);
-        std.os.exit(0);
+        os.exit(0);
     }
-    if (args.boolFlag("-version")) {
+    if (result.boolFlag("-version")) {
         try std.io.getStdOut().writeAll(@import("build_options").version);
-        std.os.exit(0);
+        os.exit(0);
     }
-    if (args.argFlag("-view-padding")) |raw| {
+    if (result.argFlag("-view-padding")) |raw| {
         view_padding = std.fmt.parseUnsigned(u32, mem.span(raw), 10) catch
-            fatal("invalid value '{s}' provided to -view-padding", .{raw});
+            fatalPrintUsage("invalid value '{s}' provided to -view-padding", .{raw});
     }
-    if (args.argFlag("-outer-padding")) |raw| {
+    if (result.argFlag("-outer-padding")) |raw| {
         outer_padding = std.fmt.parseUnsigned(u32, mem.span(raw), 10) catch
-            fatal("invalid value '{s}' provided to -outer-padding", .{raw});
+            fatalPrintUsage("invalid value '{s}' provided to -outer-padding", .{raw});
     }
-    if (args.argFlag("-main-location")) |raw| {
+    if (result.argFlag("-main-location")) |raw| {
         default_main_location = std.meta.stringToEnum(Location, mem.span(raw)) orelse
-            fatal("invalid value '{s}' provided to -main-location", .{raw});
+            fatalPrintUsage("invalid value '{s}' provided to -main-location", .{raw});
     }
-    if (args.argFlag("-main-count")) |raw| {
+    if (result.argFlag("-main-count")) |raw| {
         default_main_count = std.fmt.parseUnsigned(u32, mem.span(raw), 10) catch
-            fatal("invalid value '{s}' provided to -main-count", .{raw});
+            fatalPrintUsage("invalid value '{s}' provided to -main-count", .{raw});
     }
-    if (args.argFlag("-main-ratio")) |raw| {
+    if (result.argFlag("-main-ratio")) |raw| {
         default_main_ratio = std.fmt.parseFloat(f64, mem.span(raw)) catch
-            fatal("invalid value '{s}' provided to -main-ratio", .{raw});
+            fatalPrintUsage("invalid value '{s}' provided to -main-ratio", .{raw});
     }
 
     const display = wl.Display.connect(null) catch {
         std.debug.warn("Unable to connect to Wayland server.\n", .{});
-        std.os.exit(1);
+        os.exit(1);
     };
     defer display.disconnect();
 
@@ -404,8 +408,13 @@ fn registryListener(registry: *wl.Registry, event: wl.Registry.Event, context: *
     }
 }
 
-pub fn fatal(comptime format: []const u8, args: anytype) noreturn {
+fn fatal(comptime format: []const u8, args: anytype) noreturn {
-    const stderr = std.io.getStdErr().writer();
+    std.log.err(format, args);
-    stderr.print("err: " ++ format ++ "\n", args) catch {};
+    os.exit(1);
-    std.os.exit(1);
+}
+
+fn fatalPrintUsage(comptime format: []const u8, args: anytype) noreturn {
+    std.log.err(format, args);
+    std.io.getStdErr().writeAll(usage) catch {};
+    os.exit(1);
 }