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: autocomplete #4109

Closed
wants to merge 0 commits into from
Closed

feat: autocomplete #4109

wants to merge 0 commits into from

Conversation

HyperDanisH
Copy link

@HyperDanisH HyperDanisH commented Mar 13, 2024

What kind of change does this PR introduce?
feature #1402

Did you add tests for your changes?
No.

If relevant, did you update the documentation?
No, it is pending.

Summary
Introduces autocomplete feature for shell based command lines.

Does this PR introduce a breaking change?
No

Other information
You will have to run npm link in /packages/webpack-cli folder in order to test this feature. I am not sure how can I write tests, I need someone who can help me figure out.

Work needed (Need help from community for that)
--> Write unit tests
--> This PR as of now is just an autocomplete command setup, there is still need for add all the commands which are supported by webpack-cli. This can be done by updating autocompleteTree object in file /packages/webpack-cli/src/utils/autocomplete.ts
--> Update Docs

@HyperDanisH HyperDanisH requested a review from a team as a code owner March 13, 2024 08:37
Copy link

linux-foundation-easycla bot commented Mar 13, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@HyperDanisH HyperDanisH marked this pull request as draft March 13, 2024 08:43
@alexander-akait
Copy link
Member

Can you provide steps how it can be used

/cc @webpack/cli-team Can anyone follow up on this development?

@HyperDanisH
Copy link
Author

HyperDanisH commented Mar 13, 2024

--> You should be in a shell supported command line. Window's Command Prompt if I recall is not in that list so this might not work there. If you are using WSL (Windows Subsystem for Linux) then it is definitely supposed to work.

-->Use npm link command in packages/webpack-cli folder. You can't use this feature by running node ./bin/cli.js command. Npm link is a must.

-->After linking run webpack-cli setup-autocomplete command. This is must in order to setup the autocomplete in user's shell based command line.

-->After this when you type webpack-cli and then press tab key on keyboard, It is supposed to give suggestions. As of now it only suggest one command but it is easy to setup others in no time by writting all the commands down at autocompleteTree object in file /packages/webpack-cli/src/utils/autocomplete.ts.

{
long: "--stats",
},
],
Copy link
Member

Choose a reason for hiding this comment

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

You can get them, we already has logic where store all commands and flags

Copy link
Author

Choose a reason for hiding this comment

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

That will definitely make things more easy and maintainable. Please tell me how to get them.

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/webpack/webpack-cli/blob/master/packages/webpack-cli/src/webpack-cli.ts#L1160

You can move them to the static helper, please study the code before you start working on it

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure where can I find static helper methods. Do they exist? I seem to have not seen them in project. My apologies if I am asking a dumb doubt. I just not want things to break at all.

@webpack-bot
Copy link

Hi @HyperDanisH.

Just a little hint from a friendly bot about the best practice when submitting pull requests:

Don't submit pull request from your own master branch. It's recommended to create a feature branch for the PR.

You don't have to change it for this PR, just make sure to follow this hint the next time you submit a PR.

@HyperDanisH
Copy link
Author

WIP. Not abandoned, just a little bit busy with the GSOC2024 application. Will continue to work on it asap.

@HyperDanisH
Copy link
Author

@alexander-akait, I have come up with a way to finish autocomplete in webpack-cli. But the trade off is that there will be much refactoring needed to be done in the preexisting codebase. Am I allowed to do that. I thought I should ask before I put effort on that.

@alexander-akait
Copy link
Member

@HyperDanisH I don't think we really need to make these changes, there is commander API and you can get all commands and options there

@HyperDanisH
Copy link
Author

What do you think about this? #1402 (comment).

@HyperDanisH
Copy link
Author

After the latest commit in this PR, we can see autocomplete in action for all commands. all that left is getting the options from commander API and adding autocomplete support for them as well. and maybe some refactoring.

package.json Outdated
@@ -89,6 +89,7 @@
"sass-loader": "^13.0.2",
"strip-ansi": "^6.0.1",
"style-loader": "^3.3.1",
"test-package": "^0.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

Why we need it?

});

return autocompleteTree;
};
Copy link
Member

Choose a reason for hiding this comment

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

Let's make these function as a static method for webpack-cli class, it can be used for other features

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

@HyperDanisH could you rebase?

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

could you rebase?

@HyperDanisH
Copy link
Author

could you rebase?

On it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants