From a5a505ecbaecacbc79c515daf55ea55ab8e475f0 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Sat, 24 Jul 2021 18:29:48 +0200 Subject: [PATCH] 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 | 4 +- common/args.zig | 126 --------------------------------------------- common/flags.zig | 104 +++++++++++++++++++++++++++++++++++++ river/main.zig | 53 ++++++++----------- rivertile/main.zig | 59 ++++++++++++--------- 5 files changed, 161 insertions(+), 185 deletions(-) delete mode 100644 common/args.zig create mode 100644 common/flags.zig diff --git a/build.zig b/build.zig index b792964..8dbf0bd 100644 --- a/build.zig +++ b/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); diff --git a/common/args.zig b/common/args.zig deleted file mode 100644 index 669f9f9..0000000 --- a/common/args.zig +++ /dev/null @@ -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 . - -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 - } - }; -} diff --git a/common/flags.zig b/common/flags.zig new file mode 100644 index 0000000..0836706 --- /dev/null +++ b/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 . + +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; +} diff --git a/river/main.zig b/river/main.zig index 56a9477..6403f18 100644 --- a/river/main.zig +++ b/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 FlagDef = @import("args").FlagDef; +const flags = @import("flags"); 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 Run `sh -c ` 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| { - const log_level = std.fmt.parseInt(u3, std.mem.span(level_str), 10) catch - fatal("Error: invalid log level '{s}'", .{level_str}); + if (result.argFlag("-l")) |level_str| { + const log_level = std.fmt.parseInt(u3, std.mem.span(level_str), 10) catch { + 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; - } - } -} diff --git a/rivertile/main.zig b/rivertile/main.zig index cc977b4..59a0f6e 100644 --- a/rivertile/main.zig +++ b/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 FlagDef = @import("args").FlagDef; +const flags = @import("flags"); 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 args = Args(0, &[_]FlagDef{ + const argv: [][*:0]const u8 = os.argv; + 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 { - const stderr = std.io.getStdErr().writer(); - stderr.print("err: " ++ format ++ "\n", args) catch {}; - std.os.exit(1); +fn fatal(comptime format: []const u8, args: anytype) noreturn { + std.log.err(format, args); + 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); }