Skip to content
This repository was archived by the owner on Oct 16, 2024. It is now read-only.

Conversation

@ecairol
Copy link
Contributor

@ecairol ecairol commented Feb 3, 2024

WIP

@ecairol
Copy link
Contributor Author

ecairol commented Feb 5, 2024

@ahegyes how do you feel about this. When the user writes a command and hits TAB, besides displaying the available options and arguments, to also offer the user a full written example with placeholders?

Screenshot 2024-02-05 at 10 55 14

--
At least on my Terminal the options are not selectable, and I think it could be useful to have a reference of the value types that can be used for each option, but I'm also having second thoughts about it.

@ahegyes
Copy link
Contributor

ahegyes commented Feb 8, 2024

how do you feel about this

I'm in favor of not doing that simply because I think it kinda clutters the terminal and I haven't seen it done anywhere else.

But I can see some value in it. However, is the example auto-generated or will that be another thing to remember to update if the options change? 🤔

@ecairol
Copy link
Contributor Author

ecairol commented Feb 8, 2024

@ahegyes Yeah that's ok, it was an idea I had and wanted to hear opinions, but I wasn't 100% sold on it either. I haven't found a way to make it auto-generated so that's a con, it would need to be manually updated 👎

@ecairol
Copy link
Contributor Author

ecairol commented Feb 21, 2024

@ahegyes some recent updates:

  • I've implemented a Trait called Autocomplete, with a generic complete() function, and now all commands are using it. It works like a charm
  • Removed the "example" part from the auto-complete
  • Now the --help command skips the 1Password part and returns results much faster
  • There was a conflict with Symphony autocomplete and the -c command, so I had to rename the --contractor option to -con

Would you mind testing when you have a chance?

@ecairol ecairol force-pushed the add/symfony-autocomplete branch from 34f1392 to be3a6a0 Compare March 28, 2024 17:50
Also added int return type and changed.
@ecairol
Copy link
Contributor Author

ecairol commented Apr 10, 2024

@fmfernandes since you're already familiar with this problem and its solution, would you mind taking a look at commit eae6cb4 and let me know if you see anything suspicious or have any suggestion or question?

Copy link
Contributor

@fmfernandes fmfernandes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks good. Left a couple comments regarding some return types I'd change to match the command's output.

@ecairol
Copy link
Contributor Author

ecairol commented Apr 11, 2024

Thanks for those observations @fmfernandes, I've updated both cases here: c5978c9

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants