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

feat(z): include cwd folders in suggestions for zoxides z command #2300

Merged

Conversation

ScottRobinson03
Copy link
Contributor

@ScottRobinson03 ScottRobinson03 commented Mar 23, 2024

What

The existing spec for the z command includes folders of the cwd for rupa/z, but not zoxide's z. This PR adds autocomplete suggestions for zoxide's z command and therefore resolves aws/q-command-line-discussions#114.

I also added icons as per the following:

📁 ... = not in zoxide history, but in cwd/pwd
💾 ... = in zoxide history AND cwd/pwd
💾/... = in zoxide history, but NOT cwd/pwd (notice the leading / in path)

Example:
image

Known Issues:

For some reason I can't seem to consistently get all of zoxide's cwd suggestions above the non-zoxide cwd suggestions. I'm using the priority key to try and achieve this, but it doesn't seem to work here for some reason? Would appreciate assistance in debugging this.

image

@withfig-bot
Copy link
Collaborator

withfig-bot commented Mar 23, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@withfig-bot
Copy link
Collaborator

Overview

src/z.ts:

Info:

@withfig-bot
Copy link
Collaborator

Hello @ScottRobinson03,
thank you very much for creating a Pull Request!
Here is a small checklist to get this PR merged as quickly as possible:

  • Do all subcommands / options which take arguments include the args property (args: {})?
  • Are all options modular? E.g. -a -u -x instead of -aux
  • Have all other checks passed?

Please add a 👍 as a reaction to this comment to show that you read this.

@@ -125,9 +125,12 @@ const zoxideCompletionSpec: Fig.Spec = {
name: "directory",
filterStrategy: "fuzzy",
suggestCurrentToken: true,
isVariadic: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opted to remove this isVariadic: true since whilst the argument is strictly speaking variadic, the autocomplete for it is going to be wrong 99% of the time and so figured it's better to not give suggestions than give wrong suggestions.

@ScottRobinson03
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

withfig-bot added a commit that referenced this pull request Mar 23, 2024
@grant0417
Copy link
Member

Lgtm

@grant0417
Copy link
Member

The issue with priority not working is likely history which will result in previously selected items to be at the top even when a high priority is set, this is something I will need to look into fixing but at the moment this should be fine.

@grant0417 grant0417 merged commit 37ba7d6 into withfig:master Mar 23, 2024
5 checks passed
@withfig withfig locked and limited conversation to collaborators Mar 23, 2024
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.

None yet

3 participants