Add auto-completion #7

ammario · 2024-03-15T22:05:18Z

TODO:

Add completion handler to Command
Thoroughly test interface on coder/coder
Find a clean way to skip flag parsing when invoked for auto-completion
Find a clean way for commands to give their own auto-complete suggestions
The completion command should have an interactive install by default

Closes #3

So we can have a first-class CompletionHandler

ethanndickson · 2024-06-26T06:41:38Z

Few things to note after spending some time on this:

Are we sure we want to supply a default completion command?
I'm in favour of just exposing an API for retrieving all the info one might need to create the command.
Library users will likely want to apply their own styling if it's interactive. Even in coder/coder we want to just reuse the cliui boilerplate we have, and I'm not sure we want to recreate that here, or even import it?
To my surprise, if you disable path auto-completion in fish's complete using -f, fish just straight up ignores arguments if it thinks you're trying to autocomplete a path:
e.g, I have some uncommitted fish+go code responsible for figuring out the current word:
COMPLETION_MODE=1 CURSOR_POS=21 completetest file ../ will print files in the parent directory, using the Go file handler.
Hitting tab on completetest file ../ will not give any suggestions.
This means the protocol we use needs a way to tell the shell whether it should use it's file auto-completion. I haven't looked at the bash implementation all that much, but this is currently what Cobra does for all the shells it supports, and I do think it's better than implementing our own path completer in Go.
I'd like to get auto-completions in the middle of a line working - I'm currently passing the cursor position to serpent, but I think it'd be better to just pass the args until the currently selected character.
I wanted to keep the Go API for completions similar to the middleware API, allowing users to compose completion handler functions, but I'm not sure there's that great of a need? The only use case I can envision for composing is when you have a command that has both arguments, and a subcommand with different arguments. Then you would compose a default handler (flags + subcommands) with a custom handler (for your arguments). This doesn't sound like a super popular pattern, afaik there's nothing in coder/coder that does something like this, so I think a single handler for a Command and a single handler for an Option (that has a flag) will do.
We can then supply convenience functions for these handlers like func EnumHandler(choices ...string) CompHandler.

Let me know if you have any thoughts. I'll make a commit for feedback once I have a bare minimum prototype that I'm happy with.

ammario · 2024-06-26T15:56:45Z

Are we sure we want to supply a default completion command?
I'm in favour of just exposing an API for retrieving all the info one might need to create the command.
Library users will likely want to apply their own styling if it's interactive. Even in coder/coder we want to just reuse the cliui boilerplate we have, and I'm not sure we want to recreate that here, or even import it?

Nope, I'm game to provide the tools for people to craft their own command, but then we should put a complete example in the README.

To my surprise, if you disable path auto-completion in fish's complete using -f, fish just straight up ignores arguments if it thinks you're trying to autocomplete a path:
e.g, I have some uncommitted fish+go code responsible for figuring out the current word:
COMPLETION_MODE=1 CURSOR_POS=21 completetest file ../ will print files in the parent directory, using the Go file handler.
Hitting tab on completetest file ../ will not give any suggestions.
This means the protocol we use needs a way to tell the shell whether it should use it's file auto-completion. I haven't looked at the bash implementation all that much, but this is currently what Cobra does for all the shells it supports, and I do think it's better than implementing our own path completer in Go.

It's been a while since I was looking at this, but I came to the conclusion that it's better to disable the path completion and do it all within Go since that's pretty easy to implement and then users can easily add filters like "only accept accept directory".

I'd like to get auto-completions in the middle of a line working - I'm currently passing the cursor position to serpent, but I think it'd be better to just pass the args until the currently selected character.

I struggled with this too. I think doing middle-line completion correctly will involve a larger refactor to argument parsing. I think it makes sense, for MVP, to just prune args to cursor.

I wanted to keep the Go API for completions similar to the middleware API, allowing users to compose completion handler functions, but I'm not sure there's that great of a need? The only use case I can envision for composing is when you have a command that has both arguments, and a subcommand with different arguments. Then you would compose a default handler (flags + subcommands) with a custom handler (for your arguments). This doesn't sound like a super popular pattern, afaik there's nothing in coder/coder that does something like this, so I think a single handler for a Command and a single handler for an Option (that has a flag) will do.
We can then supply convenience functions for these handlers like func EnumHandler(choices ...string) CompHandler.

Yeah, we don't need to optimize for composition in the MVP.

Bear in mind that coder/coder is the only major user of the library, and it's OK if we make breaking changes down the road.

option.go

ethanndickson · 2024-07-12T10:34:29Z

@ammario
This should be all the core functionality, can merge once you're happy with it :)
Nice-to-haves like descriptions can come in a future PR.

command.go

ammario · 2024-07-12T19:23:14Z

command.go

+	// When invoked `WithOS`, this includes argv[0], otherwise it is the same as Args.
+	AllArgs []string
+	// CurWord is the word the terminal cursor is currently in
+	CurWord string


Can CurWord be an index into AllArgs? This provides future compatibility with mid-line completions.

To get auto-complete for equals flags (--flag=) I'm currently just setting this to empty string before we call any handlers. If we don't do this then anyone writing a flag completion handler would sometimes see --flag=<arg> as the current word, and other times just <arg>

Also, what use-case for mid-line completions isn't handled by just truncating the line at the cursor? Everything I tried just works.

Hmm, ok. If we're truncating the line at the cursor then CurWord's only purpose is to distinguish the true flag value? That makes sense, I think it warrants a comment.

command.go

completion/handlers.go

ammario · 2024-07-12T20:44:12Z

completion/handlers.go

+}
+
+func ListFiles(word string, filter func(info os.FileInfo) bool) []string {
+	out := make([]string, 0, 32)


32 initial cap seems arbitrary?

if there's a good reason it should become a comment, otherwise make this var out []string to reduce cognitive overhead.

Yeah, it's arbitrary, I just wanted to avoid allocing the array 5 times for 16 files. Assuming var out []string gives us a zero capacity: 0->1->2->4->8->16.

This is too minor to hold up the PR, but what leads you to optimize around that number of files?

Just naively based off the fact that a 0 capacity slice grows 'slowly' for the first 32 elements. Though I also could have just set the capacity to the length of the slice returned by Readdir. I'll go with just a var out []string since it's not hugely important.

completion/zsh.go

example/completetest/main.go

ammario · 2024-07-15T15:34:57Z

unfortunately GitHub doesn't let me be a "reviewer" on my own PR. On future hand-offs we should probably recreate the PR. In this one, just tag me via comment when ready for additional review.

ethanndickson · 2024-07-17T08:51:49Z

Yeah I'm happy with these changes, pls review 🙏 @ammario

ammario · 2024-07-17T16:43:06Z

command.go

+	// When invoked `WithOS`, this includes argv[0], otherwise it is the same as Args.
+	AllArgs []string
+	// CurWord is the word the terminal cursor is currently in
+	CurWord string


Hmm, ok. If we're truncating the line at the cursor then CurWord's only purpose is to distinguish the true flag value? That makes sense, I think it warrants a comment.

command.go

ammario · 2024-07-17T16:52:29Z

completion/handlers.go

+}
+
+func ListFiles(word string, filter func(info os.FileInfo) bool) []string {
+	out := make([]string, 0, 32)


This is too minor to hold up the PR, but what leads you to optimize around that number of files?

command.go

completion/handlers.go

ethanndickson · 2024-07-25T12:38:07Z

@ammario

ammario · 2024-08-01T21:38:20Z

Hey Ethan, sorry for the late review. I just skimmed the changes so we can get this in. I assume you're going to be integrating this into coder/coder next?

ethanndickson · 2024-08-02T03:29:20Z

Yep, when my backlog clears up that'll be next, serpent version bump + interactive install.

ammario added 3 commits March 15, 2024 17:00

Add some very basic scaffolding for auto completion

c103f5e

Move completion code into root package

b7749c3

So we can have a first-class CompletionHandler

Play around

ebea569

ammario mentioned this pull request Jun 18, 2024

Enable generating/providing completions #16

Closed

ethanndickson self-assigned this Jun 26, 2024

mvp + tests

d910fec

ammario commented Jul 3, 2024

View reviewed changes

option.go Outdated Show resolved Hide resolved

ethanndickson added 7 commits July 4, 2024 04:04

fixup + mid-line bash complete

707afa6

zsh support + file list completion handler

91ccb9a

fixup

6db6e6c

equals flags

3a591bf

powershell

7b640ff

refactor

5ae2c90

shorthand flags + cleanup

cff5dbb

ammario commented Jul 12, 2024

View reviewed changes

ammario requested review from ethanndickson and removed request for ethanndickson July 12, 2024 20:55

ethanndickson added 2 commits July 15, 2024 08:44

review feedback p1

aa6a8d3

review p2

15ad85c

ammario commented Jul 17, 2024

View reviewed changes

ethanndickson added 2 commits July 25, 2024 12:18

review

3fcf037

fixup

c365495

ammario merged commit 91966a2 into main Aug 1, 2024
1 check passed

ammario deleted the completion branch August 2, 2024 17:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add auto-completion #7

Add auto-completion #7

ammario commented Mar 15, 2024 •

edited

Loading

ethanndickson commented Jun 26, 2024 •

edited

Loading

ammario commented Jun 26, 2024

ethanndickson commented Jul 12, 2024 •

edited

Loading

ammario Jul 12, 2024

ethanndickson Jul 15, 2024 •

edited

Loading

ethanndickson Jul 15, 2024

ammario Jul 17, 2024

ammario Jul 12, 2024

ethanndickson Jul 15, 2024 •

edited

Loading

ammario Jul 17, 2024

ethanndickson Jul 25, 2024 •

edited

Loading

ammario commented Jul 15, 2024

ethanndickson commented Jul 17, 2024 •

edited

Loading

ammario Jul 17, 2024

ammario Jul 17, 2024

ethanndickson commented Jul 25, 2024

ammario commented Aug 1, 2024

ethanndickson commented Aug 2, 2024

Add auto-completion #7

Add auto-completion #7

Conversation

ammario commented Mar 15, 2024 • edited Loading

ethanndickson commented Jun 26, 2024 • edited Loading

ammario commented Jun 26, 2024

ethanndickson commented Jul 12, 2024 • edited Loading

ammario Jul 12, 2024

Choose a reason for hiding this comment

ethanndickson Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

ethanndickson Jul 15, 2024

Choose a reason for hiding this comment

ammario Jul 17, 2024

Choose a reason for hiding this comment

ammario Jul 12, 2024

Choose a reason for hiding this comment

ethanndickson Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

ammario Jul 17, 2024

Choose a reason for hiding this comment

ethanndickson Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

ammario commented Jul 15, 2024

ethanndickson commented Jul 17, 2024 • edited Loading

ammario Jul 17, 2024

Choose a reason for hiding this comment

ammario Jul 17, 2024

Choose a reason for hiding this comment

ethanndickson commented Jul 25, 2024

ammario commented Aug 1, 2024

ethanndickson commented Aug 2, 2024

ammario commented Mar 15, 2024 •

edited

Loading

ethanndickson commented Jun 26, 2024 •

edited

Loading

ethanndickson commented Jul 12, 2024 •

edited

Loading

ethanndickson Jul 15, 2024 •

edited

Loading

ethanndickson Jul 15, 2024 •

edited

Loading

ethanndickson Jul 25, 2024 •

edited

Loading

ethanndickson commented Jul 17, 2024 •

edited

Loading