Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add arbitrary subcommands #9

Open
SoraTenshi opened this issue Apr 30, 2023 · 10 comments
Open

Add arbitrary subcommands #9

SoraTenshi opened this issue Apr 30, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@SoraTenshi
Copy link

Hello there,

first of all, great project!
However i was wondering whether or not it would be a good addition to actually allow for arbitrary subcommands, as an example:

I have created a simple cli-tool that converts the first argument given to the program, like for example so:
./my_binary 1337 which then would parse the 1337 and i would be able to use said value.

I am not sure if this would be in scope of this project, but i think it would greatly benefit this library.
Also; i might be able to implement and contribute it myself, however i am not sure if this is in scope of this project. (hence writing an issue)

@prajwalch
Copy link
Owner

prajwalch commented May 1, 2023

Currently you can do that by using setting but I am not sure if that would solve your problem or not.

var app = App.init(allocator, "my_binary", null);
defer app.deinit();

try app.rootCommand().addArg(Arg.init("NUM", null)); 
app.rootCommand().setSetting(.takes_positional_arg);

const args = try app.parseProcess();

And yah feel free to contribute 😊

Edit: This no longer works due to the recent changes in API.

@SoraTenshi
Copy link
Author

Sorry that it's taking so long.
I noticed, to implement the anonymous arg in a way i would actually prefer, i would have to make some more generalized changes in regards as to how you'd "find" entries in the table as well as display.

What's your general take on this?
I would be relatively straight forward what to do, but it's quite a bit.

@prajwalch
Copy link
Owner

prajwalch commented Jun 6, 2023

Here is the rough idea on how we can implement that.

  1. Add a new setting for command called allow_anonymous_arg or maybe allow_anon_arg
  2. Change the implementation of parser to support parsing anonymous arg. Inside parse function
var anon = Arg.init("*", null);
anon.setSetting(.takes_multiple_values);
// To limit how many anon value user can pass we can set limit
anon.setMaxValues(10);

// To store anon values
var anon_values = std.ArrayList([]const u8).init(self.allocator);
if (self.cmd.isSettingSet(.allow_anonymous_arg)) {
    // parse values
}

// After parsing complete store its value like
self.putMatchedArg(anon, .{ .many = anon_values });
  1. Add two new API for ArgsContext, one for getting single value and another for all values.
pub fn getRawValue(self: *const ArgsContext, index: usize) ?[]const u8 {
    const anon_values = self.args.get("*") orelse return null;
    
    if (anon_values.isMany()) {
        // check for out of bound and return
    } else {
        return null;
    }
}

pub fn getAllRawValues(self: *const ArgsContext) ?[][]const u8 {
    if (self.args.get("*")) |value| {
        if (value.isMany()) return value.many.items[0..];
    }
    return null;
}

The hardest part will be identifying anon values ​​and positional arguments. The only solution I can think of is to limit how many anon values ​​the user can pass to the anon argument. If you got any other solution for this feel free to share :)

References:
https://github.com/PrajwalCH/yazap/blob/d62832cfe2100fc8d2bc55a59eb2bf1a82e94043/src/Command.zig#L11-L16
https://github.com/PrajwalCH/yazap/blob/d62832cfe2100fc8d2bc55a59eb2bf1a82e94043/src/Parser.zig#L82-L132
https://github.com/PrajwalCH/yazap/blob/d62832cfe2100fc8d2bc55a59eb2bf1a82e94043/src/Arg.zig#L25
https://github.com/PrajwalCH/yazap/blob/d62832cfe2100fc8d2bc55a59eb2bf1a82e94043/src/Arg.zig#L8-L12
https://github.com/PrajwalCH/yazap/blob/d62832cfe2100fc8d2bc55a59eb2bf1a82e94043/src/Parser.zig#L360
https://github.com/PrajwalCH/yazap/blob/d62832cfe2100fc8d2bc55a59eb2bf1a82e94043/src/args_context.zig#L50-L132
https://github.com/PrajwalCH/yazap/blob/d62832cfe2100fc8d2bc55a59eb2bf1a82e94043/src/args_context.zig#L9-L35

@SoraTenshi
Copy link
Author

in my branch i have had some idea, in which you basically print out the possible types that the anonymous arg accepts:

/// Appends a positional (non-option) into the arg list
pub fn addPositionalArg(self: *Command, comptime arg_types: anytype) !void {
    // Build a string based on the types
    comptime var type_str: []const u8 = "";
    inline for (arg_types, 0..) |t, i| {
        type_str = type_str ++ switch (t) {
            .string => "str",
            .unsigned_int => "uint",
            .signed_int => "int",
            .float => "float",
        };

        if (arg_types.len > i + 1) {
            type_str = type_str ++ "|";
        }
    }

    try self.args.append(self.allocator, Arg.init(type_str, null));
    self.setSetting(.takes_positional_arg);
}

however with this approach, i have the issue, that the name would be the type in this case.

Do you got an idea for that?
The only idea i could come up with is to essentially decide between "internal" names and what is actually printed in e.g. the help

@prajwalch
Copy link
Owner

prajwalch commented Jun 6, 2023

Currently we don't have a good API like flag to define positional arguments. Because of that there is no way to manually set the index for a positional argument, which forces consumers of the library to define them in the correct order and makes it difficult for us to handle both arbitrary values ​​and positional arguments correctly.

So to solve both problems we can add two new API, one for defining positional arguments and one for defining arbitrary arg at given index.

// Index starts with 1
var path = PositionalArg("PATH", 2);
// set settings

try root_cmd.addArg(ArbitraryArg.at(1));
try root_cmd.addArg(path);

Then the parser first parses all the value from all index registered by ArbitaryArg and PositionalArg in case of above example parser gets the value from index 0 and 1. After that it stores the value, marks that the parsing is complete for all arbitrary and positional argument and starts to parse options and subcommands.

On help message we can display like below:

Some description

USAGE: hello [$] [PATH]

ARGS:
$:          Any arbitrary value you like
PATH:       Path to read

Or maybe something like this:

Some description

USAGE: hello [...] [PATH]

ARGS:
...:        Any arbitrary value you like
PATH:       Path to read

@prajwalch prajwalch added the enhancement New feature or request label Jun 7, 2023
@prajwalch
Copy link
Owner

@SoraTenshi Hey, If you need this feature then i will implement and release it on next version :)

@SoraTenshi
Copy link
Author

@SoraTenshi Hey, If you need this feature then i will implement and release it on next version :)

I don't strictly need need it, and also I wanted to contribute :^)

But I can't find the time atm, don't cause any stress or so, I just wanna update my small cli tool (zconv) to be available at zig master, that's all ^^

@prajwalch
Copy link
Owner

@SoraTenshi Hey, If you need this feature then i will implement and release it on next version :)

I don't strictly need need it, and also I wanted to contribute :^)

This week i will be polishing and refactoring a lot of things, so you can join :)

But I can't find the time atm, don't cause any stress or so, I just wanna update my small cli tool (zconv) to be available at zig master, that's all ^^

Ok :)

@SoraTenshi
Copy link
Author

Sure i would love to.

Btw here you could see my initial 'idea' on how to implement the positional args:
main...SoraTenshi:yazap:add-arbitrary-arg

the problem was - as mentioned - that i noticed that the name would also be the display_name which i think could be a big improvement to basically store the 'find name' and 'display name' as 2 separate values.

Do you got something like discord or a like, so that you could organize the refactoring? Or maybe just create some issues, so that i can help you with the refactoring (alignment of work, i suppose) :)

@prajwalch
Copy link
Owner

Sure i would love to.

Btw here you could see my initial 'idea' on how to implement the positional args: main...SoraTenshi:yazap:add-arbitrary-arg

the problem was - as mentioned - that i noticed that the name would also be the display_name which i think could be a big improvement to basically store the 'find name' and 'display name' as 2 separate values.

Do you got something like discord or a like, so that you could organize the refactoring? Or maybe just create some issues, so that i can help you with the refactoring (alignment of work, i suppose) :)

Here's my discord id: Prajwal#7713 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants