Skip to content

feat: add npm handler#49

Merged
Aslemammad merged 10 commits intofeat/add-manual-completionsfrom
feat/add-handlers
Sep 13, 2025
Merged

feat: add npm handler#49
Aslemammad merged 10 commits intofeat/add-manual-completionsfrom
feat/add-handlers

Conversation

@AmirSa12
Copy link
Copy Markdown
Member

No description provided.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Sep 11, 2025

⚠️ No Changeset found

Latest commit: fab13ca

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Sep 11, 2025

Open in StackBlitz

npm i https://pkg.pr.new/bombshell-dev/tab/@bomb.sh/tab@49

commit: 26ed777

@AmirSa12 AmirSa12 changed the title feat: add npm handler feat: add npm and bun handlers Sep 11, 2025
@AmirSa12 AmirSa12 changed the base branch from main to feat/add-manual-completions September 11, 2025 19:55
@AmirSa12 AmirSa12 changed the title feat: add npm and bun handlers feat: add npm handler Sep 13, 2025
Comment thread bin/handlers/npm-handler.ts Outdated
(v) =>
(
({
hoisted: 'Hoist all dependencies to top level',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should never hardcode descriptions when we parse outputs. it should be either loaded from the help or it should not be there.

Comment thread bin/handlers/pnpm-handler.ts Outdated
const lines = toLines(helpText);

// edge case: reporter often appears as multiple lines
if (optionName === 'reporter') {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should never create edge cases that can be broken at any point. the algorithm here should be more general

imagine if somehow another option appeared like reporter, or reporter itself changed its structure. then this would break.

Comment thread bin/utils/shared.ts Outdated
complete('apps/*', 'All apps in apps directory');
},

registry(complete) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i don't think we should autocomplete values for this.

const patterns = getWorkspacePatterns();
patterns.forEach((p) => {
complete(p, `Workspace pattern: ${p}`);
complete(`${p}...`, `Include dependencies of ${p}`);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

are these valid?

Comment thread bin/utils/shared.ts Outdated
const patterns = getWorkspacePatterns();
patterns.forEach((p) => complete(p, `Workspace pattern: ${p}`));
complete('packages/*', 'All packages in packages directory');
complete('apps/*', 'All apps in apps directory');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

are these valid?

@Aslemammad Aslemammad merged commit 8d161a4 into feat/add-manual-completions Sep 13, 2025
2 of 3 checks passed
@Aslemammad Aslemammad deleted the feat/add-handlers branch September 13, 2025 10:46
Aslemammad pushed a commit that referenced this pull request Sep 13, 2025
…d npm handler (#48)

* init

* update

* update

* feat: add npm handler (#49)

* add npm handler

* big update

* clean npm handler

* clean shared

* clean pnpm handler

* clean bun handler

* update

* update

* ci

* update

* rm debug log
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