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

zsh autocomplete #608

Merged
merged 3 commits into from
Jul 9, 2023
Merged

zsh autocomplete #608

merged 3 commits into from
Jul 9, 2023

Conversation

jhheider
Copy link
Contributor

@jhheider jhheider commented Jun 7, 2023

This works. The places I think it might need love:

  • it will be slow. and ever slower. useSync should cache all the provides to the sqlite store for much faster performance.
  • it wasn't in any way clear to me how to interleave our completions with zsh's defaults. possibly because we don't work like anything else
    • one possibility is to do the path scanning ourselves, so we can just do the whole completion stack. this will also take time, and want for caching.

@jhheider jhheider requested a review from mxcl June 7, 2023 22:51
@what-the-diff
Copy link

what-the-diff bot commented Jun 7, 2023

PR Summary

  • Addition of complete Function in a New File
    A new file named app.complete.ts was created, which houses an asynchronous function complete(). It takes a prefix as an argument and carries out operations based on the given prefix.

  • Modification of Available Options in Help
    In the app.help.ts file, the list of available options was updated. Specifically, a new option --complete <prefix> was added and the --provides option was removed.

  • Inclusion of New Functions in app.magic.ts
    Several new functions were added to the app.magic.ts to enhance the functionality of the application. These functions check conditions, handle missing commands, generate possible names for binaries, and set up command-line completion.

  • Integration of complete Function in the Main Application
    In the app.main.ts file, the newly added complete function is imported and used in a unique case for the "complete" mode.

  • Updating the Args Type with a New Mode
    The Args type found in src/args.ts was expanded to include a new mode, "complete". A new property complete was also added to the Args interface.

  • Testing for the "complete" Mode
    Tests to ensure the correct operation of the "complete" mode were added in tests/functional/run.test.ts.

@jhheider jhheider mentioned this pull request Jun 7, 2023
6 tasks
@jhheider
Copy link
Contributor Author

jhheider commented Jun 8, 2023

Those errors are a little thorny. Looks like our zsh package might need:

runtime:
  env:
    FPATH: ${{prefix}}/functions
    MODULE_PATH: ${{prefix}}/lib/zsh/{{version.marketing}}

or similar...

@jhheider
Copy link
Contributor Author

jhheider commented Jun 8, 2023

I believe pkgxdev/pantry#2132 will fix this PR, if it's deemed an acceptable solution.

@mxcl
Copy link
Member

mxcl commented Jun 18, 2023

why is this dependent on our zsh? zsh comes with mac. Yes, our zsh should work, and linux ofc. But we should be able to add this without fixing zsh and looking at my current schedule I will not be able to fix zsh for weeks.

@jhheider
Copy link
Contributor Author

jhheider commented Jun 18, 2023

Probably because of how our tests work. I'll see if I can fix the test.

Was trying to avoid removing a test that caught a real issue to push code through.

@jhheider
Copy link
Contributor Author

pkgxdev/pantry#2132 should do it.

@jhheider
Copy link
Contributor Author

Confirmed, and zsh fixed in the bargain. Ready for critique!

@jhheider jhheider requested review from mxcl and removed request for mxcl June 18, 2023 21:07
Copy link
Member

@mxcl mxcl left a comment

Choose a reason for hiding this comment

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

Tried it and it works!

$ py
pygmentize        pyproject-build   python            
pylsp             pytest            python\{\{        
python                               python\{\{\ version.major\ \}\}
python3                              python\{\{\ version.marketing\ \}\}

needs some moustache expansion.

$ git cl
clang          clear          cluster        
clang++        clippy-driver  clusterdb      
clean -- remove untracked files from working tree
clone -- clone repository into new directory

Seems we are completing any time TAB is pressed.

.vscode/settings.json Outdated Show resolved Hide resolved

if _has_tea_magic; then
# Call \`tea --complete\` with the current word as the prefix
completions=($(tea --complete \${words[CURRENT]}))
Copy link
Member

Choose a reason for hiding this comment

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

even if we use sqlite having to use tea will add 80ms to all completion which sucks

src/app.magic.ts Outdated Show resolved Hide resolved
@mxcl
Copy link
Member

mxcl commented Jul 3, 2023

Can we have tea completions in teal?

@jhheider
Copy link
Contributor Author

jhheider commented Jul 3, 2023

Can we have tea completions in teal?

worth a try, certainly.

@jhheider
Copy link
Contributor Author

jhheider commented Jul 3, 2023

needs some moustache expansion.

Hm. Might just need to omit mustachioed variants. Otherwise, we'd need to do produce them for every version available...

Seems we are completing any time TAB is pressed.

That we should not do. Probably something I missed about detecting context.

@jhheider
Copy link
Contributor Author

jhheider commented Jul 3, 2023

the docs for zsh completion are long but not amazing.

@jhheider
Copy link
Contributor Author

jhheider commented Jul 3, 2023

colorizing the results eluded me. i believe everything else is addressed.

@jhheider jhheider requested a review from mxcl July 3, 2023 20:18
@mxcl
Copy link
Member

mxcl commented Jul 6, 2023

colorizing the results eluded me

was a nice to have, not required. good job trying tho

@mxcl
Copy link
Member

mxcl commented Jul 9, 2023

So trying it out, being able to distinguish between what is on the system and what is not yet on the system is pretty important IMO.

We either need coloration or to make the results appear completely separately to system results

@jhheider
Copy link
Contributor Author

jhheider commented Jul 9, 2023

Zsh completion has a grouping system. I wasn't able to properly group output (complicated, like I said), but supposedly it can be done by tags, then print a header for each section.

I go back and forth about the import. If the point is to be magical, then not showing the difference seems like the right way. If they cared about local vs tea installed, they wouldn't use magic, right?

@mxcl
Copy link
Member

mxcl commented Jul 9, 2023

I go back and forth about the import. If the point is to be magical, then not showing the difference seems like the right way. If they cared about local vs tea installed, they wouldn't use magic, right?

it's a good point.

Let's just merge and see how it feels for a few weeks I guess.

@mxcl mxcl merged commit a07eb57 into main Jul 9, 2023
5 checks passed
@mxcl mxcl deleted the zsh-autocomplete branch July 9, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants