Skript icon indicating copy to clipboard operation
Skript copied to clipboard

Add command argument tab complete

Open TheLimeGlass opened this issue 1 year ago • 11 comments

Description

Adds command parse context tab complete, and cleaned up Argument class. Takes advantage of WrapperExpression to make this extremely easy and simple to implement.

command /example <string=%tab completions "example" and "another example"%>:
	trigger:
		message string argument # messages "example" and "another example" if just /example (existing behaviour)
		if the string argument is "example":
			message "1" to player
		else if the string argument is "another example":
			message "2" to player

image


Target Minecraft Versions: any Requirements: none Related Issues: none

TheLimeGlass avatar Feb 11 '24 00:02 TheLimeGlass

Haven't yet got through the code but as an overview, I am not a big fan of the curernt way tab completions are implemented, it gives a vibe that command lines will be very long which we know the parser doesn't like that very much also it's not much organized.

What I think is better (but there are probably better solutions)

command /example <string> <string>:
	tab completion:
		something # for arg 1
		someotherthing, someotherthing2 # for arg 2
	trigger:
		if arg-1 is "arg1":
			message "1" to player
		else if arg-2 is "arg2aswell":
			message "2" to player

or we can make it even more customizable but a little more complex

command /example <string> <string>:
	tab completion:
		argument 1:
			if sender has permission "some.permission":
				return ("something", "something2")
	trigger:
		if arg-1 is "arg1":
			message "1" to player
		else if arg-2 is "arg2aswell":
			message "2" to player

AyhamAl-Ali avatar Feb 11 '24 14:02 AyhamAl-Ali

Haven't yet got through the code but as an overview, I am not a big fan of the curernt way tab completions are implemented, it gives a vibe that command lines will be very long which we know the parser doesn't like that very much also it's not much organized.

What I think is better (but there are probably better solutions)

command /example <string> <string>:
	tab completion:
		something # for arg 1
		someotherthing, someotherthing2 # for arg 2
	trigger:
		if arg-1 is "arg1":
			message "1" to player
		else if arg-2 is "arg2aswell":
			message "2" to player

or we can make it even more customizable but a little more complex

command /example <string> <string>:
	tab completion:
		argument 1:
			if sender has permission "some.permission":
				return ("something", "something2")
	trigger:
		if arg-1 is "arg1":
			message "1" to player
		else if arg-2 is "arg2aswell":
			message "2" to player

Ya absolutely, that's what Sovdee and I had planned for the future in another pull request. This allows for literal implementation directly in the argument, because Skript is setup to read tab completes from the custom Argument class. This simply adds on to it. Skript parses the default expression string input, so it's not taking the whole command pattern into consideration at a time.

TheLimeGlass avatar Feb 12 '24 04:02 TheLimeGlass

I'm strongly opposed to this implementation of tab completions. I think it's quite limiting for the user, quite confusing as it occupies the same space as default values (What if i want the default to be "help", but to have tab completes for "test" "help" and "exit"?), and just really hard to read in the actual code. I think tab completions should be properly implemented with a future pull request instead of using this, even if it's meant to be temporary (?).

sovdeeth avatar Feb 17 '24 21:02 sovdeeth

I'm strongly opposed to this implementation of tab completions. I think it's quite limiting for the user, quite confusing as it occupies the same space as default values (What if i want the default to be "help", but to have tab completes for "test" "help" and "exit"?), and just really hard to read in the actual code. I think tab completions should be properly implemented with a future pull request instead of using this, even if it's meant to be temporary (?).

You place "help" as the first index.

TheLimeGlass avatar Feb 19 '24 19:02 TheLimeGlass

You place "help" as the first index.

Your example indicates that all the tab completions would be the default value: message string argument # messages "example" and "another example" if just /example (existing behaviour)

sovdeeth avatar Feb 19 '24 19:02 sovdeeth

Your example indicates that all the tab completions would be the default value: message string argument # messages "example" and "another example" if just /example (existing behaviour)

The way string parsing works in commands, is that it'll merge multiple strings into a single string and message that. When a string argument is required but not provided, it needs to default to a single value obviously. So the tab complete wrapper expression has to default to a single value, so I made it grab the first index of the tab completions.

TheLimeGlass avatar Feb 19 '24 19:02 TheLimeGlass

Your example indicates that all the tab completions would be the default value: message string argument # messages "example" and "another example" if just /example (existing behaviour)

The way string parsing works in commands, is that it'll merge multiple strings into a single string and message that. When a string argument is required but not provided, it needs to default to a single value obviously. So the tab complete wrapper expression has to default to a single value, so I made it grab the first index of the tab completions.

I would suggest you update the example, then. Even given that, I don't think that's very intuitive or a good design at all. Again, I would rather have this implemented properly with the user have the control and flexibility they should have from the start, not an possibly temporary half-implementation in the mean time. I liked that second option you suggested on the discord, and Ayham's refinement even more, and I would prefer either of those over this.

sovdeeth avatar Feb 19 '24 19:02 sovdeeth

This is not a temporary solution. This is a solution to adding tab completions directly after writting a text parameter.

TheLimeGlass avatar Feb 19 '24 19:02 TheLimeGlass

Haven't yet got through the code but as an overview, I am not a big fan of the curernt way tab completions are implemented, it gives a vibe that command lines will be very long which we know the parser doesn't like that very much also it's not much organized. What I think is better (but there are probably better solutions)

command /example <string> <string>:
	tab completion:
		something # for arg 1
		someotherthing, someotherthing2 # for arg 2
	trigger:
		if arg-1 is "arg1":
			message "1" to player
		else if arg-2 is "arg2aswell":
			message "2" to player

or we can make it even more customizable but a little more complex

command /example <string> <string>:
	tab completion:
		argument 1:
			if sender has permission "some.permission":
				return ("something", "something2")
	trigger:
		if arg-1 is "arg1":
			message "1" to player
		else if arg-2 is "arg2aswell":
			message "2" to player

Ya absolutely, that's what Sovdee and I had planned for the future in another pull request. This allows for literal implementation directly in the argument, because Skript is setup to read tab completes from the custom Argument class. This simply adds on to it. Skript parses the default expression string input, so it's not taking the whole command pattern into consideration at a time.

I would say something like the above completely supersedes the implementation in this PR, basically making this version obsolete, and effectively temporary. My final word here is that I'd much rather see something like the above added than the implementation in this pr.

sovdeeth avatar Feb 19 '24 20:02 sovdeeth

I would say something like the above completely supersedes the implementation in this PR, basically making this version obsolete, and effectively temporary. My final word here is that I'd much rather see something like the above added than the implementation in this pr.

What about having static text arguments.

command /minigame start/stop/list/info/add [<player>]:
    trigger:
        if the text argument is "add":
            player argument is set
            add player argument to {minigame::players::*}

Then you would instinctively know the tab completions, because this is what tab completions like this are needed for. Skript would error with default error or usage if defined like how implementation currently works. If the first argument is not from the list. (can be single)

And if no argument is presents there could be default expressions:

command /minigame start/stop/list/info/add="no argument" [<player>]:
    trigger:
        if the text argument is "no argument":
            message "/minigame stop - stop the minigame"
            message "/minigame start - start the minigame"
            message "/minigame list - list the minigame arenas"
            message "/minigame info - info about the minigame"
            message "/minigame add <player> - Add a player to the minigame"

Trying to find a solution rather than just getting "no" for innovation that I find helpful for not just myself.

TheLimeGlass avatar Feb 19 '24 22:02 TheLimeGlass

command /minigame start/stop/list/info/add [<player>]:
    trigger:
        if the first text argument is "add":
            player argument is set
            add player argument to {minigame::players::*}

Then you would instinctively know the tab completions, because this is what tab completions like this are needed for. Skript would error if the first argument is not from the list. (can be single)

And if no argument is presents there could be default expressions:

command /minigame start/stop/list/info/add="no argument" [<player>]:
    trigger:
        if the first text argument is "no argument":
            message "/minigame stop - stop the minigame"
            message "/minigame start - start the minigame"
            message "/minigame list - list the minigame arenas"
            message "/minigame info - info about the minigame"

Trying to find a solution rather than just getting "no" for innovation that I find helpful for not just myself.

Sure, that could be neat. I think it'd be best without default arguments and with a clear delineator, though:

command /minigame [(start|stop|list|info|add)] [<player>]:

However, I am still of the opinion that we need a proper dynamic tab complete solution to fill the gaps, but this static implementation would fit more nicely with such a solution.

Edit: this also might be a good time to add tab completions for other literal arguments, like

command /test first [second] third:

But that may be better off waiting for a better command data structure with the rework. Up to you!

sovdeeth avatar Feb 19 '24 22:02 sovdeeth