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

Complete all keywords and builtins #44

Merged

Conversation

EmilyGraceSeville7cf
Copy link
Contributor

@EmilyGraceSeville7cf EmilyGraceSeville7cf commented Dec 27, 2023

image

@bmalehorn
Copy link
Owner

Hey @EmilyGraceSeville7cf , thanks for the PR!

I checked out your branch and it looks like the problem is you're registering it for the plaintext language, not fish. With the following change I could get suggestions working in vscode:

image

diff --git a/src/extension.ts b/src/extension.ts
index 0e8129f..3aeea40 100644
--- a/src/extension.ts
+++ b/src/extension.ts
@@ -62,7 +62,7 @@ export const activate = async (context: ExtensionContext): Promise<any> => {
   );
 
   context.subscriptions.push(
-    vscode.languages.registerCompletionItemProvider("plaintext", {
+    vscode.languages.registerCompletionItemProvider("fish", {
       provideCompletionItems(
         document: vscode.TextDocument,
         position: vscode.Position,
@@ -73,6 +73,8 @@ export const activate = async (context: ExtensionContext): Promise<any> => {
           new vscode.CompletionItem("if", vscode.CompletionItemKind.Keyword),
           new vscode.CompletionItem("while", vscode.CompletionItemKind.Keyword),
           new vscode.CompletionItem("for", vscode.CompletionItemKind.Keyword),
+          new vscode.CompletionItem("bmalehorn_was_here", vscode.CompletionItemKind.Keyword),
+
         ];
       },
     }),

@EmilyGraceSeville7cf
Copy link
Contributor Author

@bmalehorn, now completion for all keywords and builtins is done.

@EmilyGraceSeville7cf EmilyGraceSeville7cf changed the title Complete some keywords Complete all keywords and builtins Jan 4, 2024
@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as draft January 6, 2024 12:16
@EmilyGraceSeville7cf
Copy link
Contributor Author

EmilyGraceSeville7cf commented Jan 6, 2024

I will add documentation for some commands.

@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as ready for review January 6, 2024 13:28
Copy link
Owner

@bmalehorn bmalehorn left a comment

Choose a reason for hiding this comment

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

This is really cool, didn't know you could do all this!

I'm guessing to generate some of this you copy/pasted the list of builtins & descriptions from here: https://fishshell.com/docs/current/commands.html

For the arguments with options, did you write it by hand?

I'm probably just going to merge this as-is, but I may later change how this documentation is generated. I'm definitely keeping around the completions & documentation in some form, though.

I'm thinking in the future I may just show the official documentation from the fish project, which is available here: https://github.com/fish-shell/fish-shell/tree/23a8967ecbeabf656440528d520517a7491de738/doc_src/cmds
I might write a script that fetches all the .rst files, converts them to markdown, and uses that for the documentation. That means we don't need all this markdown-writing code or documentation in the source code, and updating for newer versions of fish is trivial.

Anyway I left one nit, mind addressing that and then I can merge?

src/extension.ts Outdated
],
false,
),
Builtin("else", "Execute command if a condition is not met"),
Copy link
Owner

Choose a reason for hiding this comment

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

nit: this should be Keyword

@EmilyGraceSeville7cf
Copy link
Contributor Author

EmilyGraceSeville7cf commented Jan 12, 2024

For the arguments with options, did you write it by hand?

Yeah. :)

Anyway I left one nit, mind addressing that and then I can merge?

It would be nice to have this PR merged.

@bmalehorn bmalehorn merged commit b7f550d into bmalehorn:master Jan 16, 2024
1 check passed
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.

2 participants