-
Notifications
You must be signed in to change notification settings - Fork 4
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: function to return content of completion script #11
Conversation
5db604c
to
a8f2c87
Compare
- '14' | ||
node: | ||
- '20' | ||
- '21' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it will be shipped to pnpm v9
at least v18 should be supported
lib/index.js
Outdated
if (!completer) throw new TypeError('options.completer is required'); | ||
if (!shell) throw new TypeError('options.shell is required'); | ||
const completionScriptContent = await installer.getCompletionScript({ name, completer, shell }); | ||
console.log(completionScriptContent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would make more sense to return the output that pnpm would print out.
I don't get it. The completion script is written to the fs, then read and written to the output? Why is it not generated in memory instead and returned from memory? |
The completion script must be exist in the filesystem as a file for the shell programs to load. But what pnpm/pnpm#3083 requests is for the user to install it themselves, because letting pnpm guesses the system can be error-prone. |
But the referenced issue is exactly about not writing to the fs anything. |
It is about pnpm not writing to fs for the users. The users themselves must write to fs. |
I misunderstood the |
It was a terrible name. Fortunately, I wasn't deceived (somehow). |
No description provided.