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

npm-profile, npm-registry-fetch, emit input.start #7457

Merged
merged 4 commits into from
May 2, 2024

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Apr 30, 2024

Note: this relies on npm/npm-profile#131 This now includes npm-profile@10.0.0

This PR accomplishes the following:

  • wraps the help output and url openers in input.start so progress won't be shown while the terminal is running those commands
  • Moves otplease to auth.js. They share many of the same requires and all the commands that required auth.js also required otplease
  • Combines open-url and open-url-prompt files
  • Removes web-auth in favor of a new webAuthOpener method exported from npm-profile
  • Uses readline/promises for url prompts which can directly accept the new AbortSignal API from npm-profile

@lukekarrys lukekarrys requested a review from a team as a code owner April 30, 2024 22:31
@lukekarrys lukekarrys force-pushed the lk/help-and-url-input-start branch 2 times, most recently from f66808b to 5b230c7 Compare May 1, 2024 01:38
@lukekarrys lukekarrys marked this pull request as draft May 1, 2024 01:39
@lukekarrys lukekarrys force-pushed the lk/help-and-url-input-start branch 3 times, most recently from 8aac32d to 2f08cd7 Compare May 1, 2024 01:46
@lukekarrys lukekarrys marked this pull request as ready for review May 1, 2024 06:16
@lukekarrys lukekarrys force-pushed the lk/help-and-url-input-start branch from 2f08cd7 to 19175d8 Compare May 2, 2024 16:03
@lukekarrys lukekarrys force-pushed the lk/help-and-url-input-start branch from 19175d8 to 531c674 Compare May 2, 2024 16:15
@lukekarrys lukekarrys changed the title fix: run input.start around help and openining urls npm-profile, npm-registry-fetch, emit input.start May 2, 2024
@@ -95,13 +95,15 @@ class Help extends BaseCommand {
args = ['emacsclient', ['-e', `(woman-find-file '${man}')`]]
}

return spawn(...args, { stdio: 'inherit' }).catch(err => {
Copy link
Member

Choose a reason for hiding this comment

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

npm was not using the return data from promise-spawn here so this is ok

https://github.com/npm/promise-spawn/blob/0aaf6f0f37e5b468277c8a4d8200b5ab18a4b387/lib/index.js#L27-L33

@@ -63,7 +63,7 @@ t.test('open bugs urls & emails', async t => {
const { npm } = await loadMockNpm(t, {
mocks: {
pacote,
'{LIB}/utils/open-url.js': openUrl,
'{LIB}/utils/open-url.js': { openUrl },
Copy link
Member

Choose a reason for hiding this comment

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

in the future we should use spawk for these

t.same(joinedOutput(), '', 'printed no output')
})
const { openUrlPrompt } = tmock(t, '{LIB}/utils/open-url.js', {
'@npmcli/promise-spawn': {
Copy link
Member

Choose a reason for hiding this comment

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

in the future we should use spawk here.

This also refactors auth and prompt related files to use
promises and new npm-profile behavior
@lukekarrys lukekarrys force-pushed the lk/help-and-url-input-start branch from 9c5075c to 8f73741 Compare May 2, 2024 16:38
@lukekarrys lukekarrys merged commit 8ded848 into latest May 2, 2024
144 of 145 checks passed
@lukekarrys lukekarrys deleted the lk/help-and-url-input-start branch May 2, 2024 16:50
@github-actions github-actions bot mentioned this pull request May 2, 2024
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