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

Allow parsing of multiple args for a command #102

Open
selmaohneh opened this issue Sep 25, 2018 · 4 comments
Open

Allow parsing of multiple args for a command #102

selmaohneh opened this issue Sep 25, 2018 · 4 comments
Milestone

Comments

@selmaohneh
Copy link

Allow stuff like

menu.Add("greet", (x,y) => Greet(x,y), "Greets a name a number of times.");

Would be great to have multiple parsed arguments instead of a single string.

@nerai
Copy link
Owner

nerai commented Sep 25, 2018

Do I understand this correctly: You want to add a menu entry. When this entry is called like

$ greet Jon 4

It should call the action body with the two parameters Jon and 4, i.e. it should call

`Greet("Jon","4")`

Is that right?

I think the only generic approach to this would be to add something like a regex, that parses the argument and fills in the varargs of the action body. This could probably be done in the library itself. But why not just parse the regex in the action body directly? You have full control then.

That is to say, I would probably do it like this:

menu.Add ("greet", s => {
	var regex = new Regex (@"(?<name>[^ ]*) (?<times>[0-9]*)");
	var match = regex.Match (s);
	var name = match.Groups["name"].Value;
	var times = int.Parse (match.Groups["times"].Value);
	for (int i = 0; i < times; i++) {
		Console.WriteLine ($"Hello {name}!");
	}
}, "Greets a name a number of times.");	

$ greet Jon 3
Hello Jon!
Hello Jon!
Hello Jon!

nerai added a commit that referenced this issue Sep 25, 2018
@selmaohneh
Copy link
Author

selmaohneh commented Sep 26, 2018

You understand it right. :) In addition I would even like to call Greet("Jon", 4), i.e. with the second value directly parsed to an int. It would be very convinient to have the parsing done in the library instead of writing it everytime on my own. Currently I do that in combintation with PowerArgs. Having both of your libraries work together feels like a custom git bash with my own commands :) . So, in summary:

Yeah, it works your way, but it is not that convinient. Maybe think about adding a argument parsing similar to PowerArgs.

Let me know your decision.

EDIT:

I know this has no high priority. In addition I think it is not that trivial, since it would involve reflection stuff. Maybe using PowerArgs in combination is the right way... I don't want to invent the wheel again, you don't want the dependency to PowerArgs, I guess...

@nerai
Copy link
Owner

nerai commented Sep 26, 2018

I did not know about PowerArgs, it looks like a nice library. Thanks for showing! I see some synergy in adding it here. However, it is also a significant change and dependency that I need to examine in detail. Currently I sadly lack the time, so I want to postpone this for now. Sorry!

For your use case, I'm not sure if it is possible, but what comes to mind is deriving from CMenuItem. This subclass could then implement PowerArgs, and you could create instances from it. Is that possible with PowerArgs? If your menu items are large anyway, this would probably be easier to maintain, too.

What sadly does not work at the moment is using the predefined Add-style methods like menu.Add ("x", ...) in combination with PowerArgs. Deriving from CMenuItem is required.

However, I'll add a factory method in the next patch. Then your own class gets constructed when you call Add. Maybe that helps with this effort.

nerai added a commit that referenced this issue Sep 26, 2018
@nerai
Copy link
Owner

nerai commented Sep 26, 2018

It's in the master branch. I'll push it to Nuget later, too.

@nerai nerai added this to the 0.10 milestone Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants