SpongeAPI icon indicating copy to clipboard operation
SpongeAPI copied to clipboard

Add option to only tabcomplete first alias of a child command

Open BrainStone opened this issue 6 years ago • 5 comments

Hi there, I've been told that there currently isn't a suitable way to create a child command where not all aliases are tabcompleteable.

Now why is that an issue for me?

Let's have at this simplified snippet from one of my plugins:

CommandSpec.builder()
    .description(Text.of("Base command for the plugin. Does nothing on its own."))
    .child(CommandReload.create(), "reload", "r", "rl")
    .child(CommandShow.create(), "show", "s")
    .build();

Now if I were to type the command /command <tab> I would get 5 options at that point even though there are only two subcommands. It's also annyoing that when I have /command r<tab> that instead of completing to reload it shows me the three options r, rl, reload.

Now while these things are essentially just minor inconvinieces, it would be really cool to have a simple way to clean up the CLI and have an option that allows to only tabcomplete the first alias, so that we can still provide these shortcuts without messing up tabcomplete.

I propose this interface (excpet the actual name for the boolean parameter):

public Builder child(CommandCallable child, String... aliases) {
    return child(child, false, aliases);
}

public Builder child(CommandCallable child, boolean tabCompleteFirstAliasOnly, String... aliases) {
    // Implementation
}

public Builder child(CommandCallable child, Collection<String> aliases) {
    return child(child, false, aliases);
}

public Builder child(CommandCallable child, boolean tabCompleteFirstAliasOnly, Collection<String> aliases) {
    // Implementation
}

BrainStone avatar Mar 01 '19 22:03 BrainStone

I know @xDotDash said he'd look at this but I asked him to wait for me to make a comment first. At the time I was unavailable to make a longer comment, but I've finally gotten around to it, sorry it's taken some time.

I'm iffy on this for a few reasons:

  • In API 8 we're completely reworking the command API to enable integration with Mojang's system, so whatever we do here has no guarantee of porting over. In fact, I expect that it'll become harder and against the spirit of what Brig does and I don't want to promise an API in 7.2 that we can't realise in 8.
  • As you might have gathered, child commands are a mess in 7 anyway and sometimes I feel like you only have to do so much as look at them to cause them to self-destruct. Now, that's not particularly a reason to not touch it, but because commands are API implemented in API 7 we have to take extra care in ensuring things don't go awry - making seemingly simple changes has burnt me before.
  • Tab completion is about discovery. I can use it to discover command arguments and the shortcuts that you might provide by simply pressing tab. Why remove that discovery? If you want users to use "reload", then remove the shorthand and rely on their tab behaviour. If you want to expose "r", then let them tab to it. Used incorrectly, this can really hinder the UX that users expect - so I think making it a little harder for plugins to do this will deter those who just see an API and go "ooh, look at that, let's flip that on".

I'm not going to stop you from doing it in your own plugin, and I'm sure you'll want to argue for this issue and I understand that. However, taking a broader view of this makes me think this isn't the right thing to do.


To look at your proposal, what about if I just wanted to not tab complete r, but I did want to tab complete re? Or if I had reload and rebuild and change as aliases? We'd need a way to turn off individual aliases - at that point, you're better off calling child twice with some tabcomplete boolean flipped off for the second one. But then you have to call child twice for the same command - and that's not intuitive either.

Sponge is about more than me though, so I'm happy for other opinions, but I just don't think this is something we should do.

dualspiral avatar Mar 08 '19 15:03 dualspiral

While knowing aliases is useful, having tabcomplete only show selected aliases would be useful as it would reduce clutter for less experienced users.

Pulverizer avatar Mar 14 '19 10:03 Pulverizer

You made a few fair points and I understand your opinion.
It doesn't really matter how, but all I really want is being able to hide aliases to declutter tab completion. And I'm aware that the whole system will be changed for API 8.

BrainStone avatar Mar 16 '19 07:03 BrainStone

@dualspiral status on this with new command API in api-8?

Zidane avatar May 17 '19 13:05 Zidane

Not on the radar.

Simple fact of the matter is that client completions complicate this. I expect an overwhelming number of plugins to want to use client completions and if you use a "hidden" alias then the subsequent completions for elements after the hidden aliases aren't going to work because if we send them, the client is going to display them, I see no way to say "this exists but don't show it please". The only way to not show it is to not send that part of the tree.

However, with the way client completions work in 8, I see this being less of a problem as the clutter is moved from the CLI to the hints that are displayed above it.

When I have implementation down and working, if any clever way to do this appears then maybe it'll get done.

dualspiral avatar May 17 '19 14:05 dualspiral