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

[DOCS] npm search --long flag has no effect #7434

Closed
2 tasks done
joshbax189 opened this issue Apr 27, 2024 · 5 comments · Fixed by #7441
Closed
2 tasks done

[DOCS] npm search --long flag has no effect #7434

joshbax189 opened this issue Apr 27, 2024 · 5 comments · Fixed by #7441
Assignees
Labels
Documentation documentation related issue Needs Triage needs review for next steps Release 10.x

Comments

@joshbax189
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

This could also be a feature request or a docs update depending how you look at it...

When calling npm search with the --long flag, e.g.

> npm search --long --json --searchlimit 1 color

I get this output, which is identical to calling search without --long

[
  {
    "name": "color",
    "scope": "unscoped",
    "version": "4.2.3",
    "description": "Color conversion and manipulation with CSS string support",
    "keywords": [
      "color",
      "colour",
      "css"
    ],
    "date": "2022-04-05T09:15:16.379Z",
    "links": {
      "npm": "https://www.npmjs.com/package/color",
      "homepage": "https://github.com/Qix-/color#readme",
      "repository": "https://github.com/Qix-/color",
      "bugs": "https://github.com/Qix-/color/issues"
    },
    "publisher": {
      "username": "qix",
      "email": "josh@junon.me"
    },
    "maintainers": [
      {
        "username": "qix",
        "email": "josh@junon.me"
      }
    ]
  }
]

Expected Behavior

I expected more information, perhaps the full output of registry search

For example

> npm search --long --json --searchlimit 1 color
[
  {
    "package": {
      "name": "color",
      "scope": "unscoped",
      "version": "4.2.3",
      "description": "Color conversion and manipulation with CSS string support",
      "keywords": [
        "color",
        "colour",
        "css"
      ],
      "date": "2022-04-05T09:15:16.379Z",
      "links": {
        "npm": "https://www.npmjs.com/package/color",
        "homepage": "https://github.com/Qix-/color#readme",
        "repository": "https://github.com/Qix-/color",
        "bugs": "https://github.com/Qix-/color/issues"
      },
      "publisher": {
        "username": "qix",
        "email": "josh@junon.me"
      },
      "maintainers": [
        {
          "username": "qix",
          "email": "josh@junon.me"
        }
      ]
    },
    "flags": {
      "insecure": 0
    },
    "score": {
      "final": 0.38903615890331245,
      "detail": {
        "quality": 0.5395409662759634,
        "popularity": 0.4959066747321972,
        "maintenance": 0.15316152246929846
      }
    },
    "searchScore": 100000.43
  }
]

Steps To Reproduce

Run the commands above. I've checked on 8.x, 9.x and 10.6.0 using nvm

It looks like the config value is never used in search.js. Compare the other referenced commands from the help docs: ls.js and help-search.js. Both access the config via this.npm.config.get('long'), but search.js only uses this.npm.flatOptions which doesn't contain the long prop.

Environment

  • npm: 10.6.0
  • Node.js: 20.12.2
  • OS Name: ArchLinux
  • System Model Name: Toaster
  • npm config:
; nothing special
@joshbax189 joshbax189 added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Apr 27, 2024
@joshbax189
Copy link
Author

I recovered the "expected" behavior with only a few small changes to search.js (marked in comments)

class Search extends BaseCommand {
// ...
  async exec (args) {
    const opts = {
      ...this.npm.flatOptions,
      ...this.npm.flatOptions.search,
      detailed: true, // NEW: request full response from search API
      include: args.map(s => s.toLowerCase()).filter(s => s),
      exclude: this.npm.flatOptions.search.exclude.split(/\s+/),
    }

    // NEW: long is not in flatOptions
    const returnLong = this.npm.config.get('long');

    if (opts.include.length === 0) {
      throw new Error('search must be called with arguments')
    }

    // Used later to figure out whether we had any packages go out
    let anyOutput = false

    class FilterStream extends Minipass {
      constructor () {
        super({ objectMode: true })
        console.log(opts);
      }

      write (pkg) {
        // NEW: pkg now contains a package prop plus extra info
        if (filter(pkg.package, opts.include, opts.exclude)) {
          // NEW: only JSON output supports extra fields
          super.write((returnLong && opts.json) ? pkg : pkg.package)
        }
      }
    }
// ... class continues
}

This would add support for --long when using --json, but not otherwise. It would be helpful to have access to the popularity and flags.insecure props from the CLI ;)

On the other hand, it would also make loads of sense to just remove the --long flag from search altogether and update the docs appropriately.

@wraithgar
Copy link
Member

I thing --long probably doesn't make sense anymore. It was a bandage over the real problem, which is that table output for search was not a good fit. All it ever actually did was prevent the columns from truncating, which it looks like it hasn't been doing since npm@6.

As far as adding those flags, that'd be a separate feature request. I don't thing popularity is likely to be added, but I think adding an extra line of caution if insecure is not 0 would be a great addition to the search results.

@wraithgar
Copy link
Member

Closing this as it's not a bug, but if someone wants to submit a PR to add something if insecure is true that would probably be an easy task.

@wraithgar wraithgar changed the title [BUG] npm search --long flag has no effect [DOCS] npm search --long flag has no effect Apr 28, 2024
@wraithgar wraithgar added Documentation documentation related issue and removed Bug thing that needs fixing labels Apr 28, 2024
@wraithgar
Copy link
Member

Reopening cause we should remove this from the params

@wraithgar wraithgar reopened this Apr 28, 2024
@wraithgar wraithgar self-assigned this Apr 30, 2024
wraithgar added a commit that referenced this issue Apr 30, 2024
`--long` does not do anything and hasn't in some time.
The first search param is also required.

Closes: #7402
Closes: #7434
wraithgar added a commit that referenced this issue Apr 30, 2024
`--long` does not do anything and hasn't in some time.
The first search param is also required.

Closes: #7402
Closes: #7434
@joshbax189
Copy link
Author

Nice job :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation documentation related issue Needs Triage needs review for next steps Release 10.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants