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

fix: use Intl.Collator for string sorting when available #3809

Merged
merged 1 commit into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const semver = require('semver')
const BaseCommand = require('./base-command.js')
const npa = require('npm-package-arg')
const jsonParse = require('json-parse-even-better-errors')
const localeCompare = require('@isaacs/string-locale-compare')('en')

const searchCachePackage = async (path, spec, cacheKeys) => {
const parsed = npa(spec)
Expand Down Expand Up @@ -212,10 +213,10 @@ class Cache extends BaseCommand {
for (const key of keySet)
results.add(key)
}
[...results].sort((a, b) => a.localeCompare(b, 'en')).forEach(key => this.npm.output(key))
[...results].sort(localeCompare).forEach(key => this.npm.output(key))
return
}
cacheKeys.sort((a, b) => a.localeCompare(b, 'en')).forEach(key => this.npm.output(key))
cacheKeys.sort(localeCompare).forEach(key => this.npm.output(key))
}
}

Expand Down
5 changes: 3 additions & 2 deletions lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const writeFile = promisify(fs.writeFile)
const { spawn } = require('child_process')
const { EOL } = require('os')
const ini = require('ini')
const localeCompare = require('@isaacs/string-locale-compare')('en')

// take an array of `[key, value, k2=v2, k3, v3, ...]` and turn into
// { key: value, k2: v2, k3: v3 }
Expand Down Expand Up @@ -209,7 +210,7 @@ class Config extends BaseCommand {
; Configs like \`//<hostname>/:_authToken\` are auth that is restricted
; to the registry host specified.

${data.split('\n').sort((a, b) => a.localeCompare(b, 'en')).join('\n').trim()}
${data.split('\n').sort(localeCompare).join('\n').trim()}

;;;;
; all available options shown below with default values
Expand Down Expand Up @@ -238,7 +239,7 @@ ${defData}
if (where === 'default' && !long)
continue

const keys = Object.keys(data).sort((a, b) => a.localeCompare(b, 'en'))
const keys = Object.keys(data).sort(localeCompare)
if (!keys.length)
continue

Expand Down
3 changes: 2 additions & 1 deletion lib/help.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const path = require('path')
const openUrl = require('./utils/open-url.js')
const { promisify } = require('util')
const glob = promisify(require('glob'))
const localeCompare = require('@isaacs/string-locale-compare')('en')

const BaseCommand = require('./base-command.js')

Expand Down Expand Up @@ -82,7 +83,7 @@ class Help extends BaseCommand {
if (aManNumber !== bManNumber)
return aManNumber - bManNumber

return a.localeCompare(b, 'en')
return localeCompare(a, b)
})
const man = mans[0]

Expand Down
4 changes: 2 additions & 2 deletions lib/ls.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const _problems = Symbol('problems')
const _required = Symbol('required')
const _type = Symbol('type')
const ArboristWorkspaceCmd = require('./workspaces/arborist-cmd.js')
const localeCompare = require('@isaacs/string-locale-compare')('en')

class LS extends ArboristWorkspaceCmd {
/* istanbul ignore next - see test/lib/load-all-commands.js */
Expand Down Expand Up @@ -503,8 +504,7 @@ const augmentNodesWithMetadata = ({
return node
}

const sortAlphabetically = (a, b) =>
a.pkgid.localeCompare(b.pkgid, 'en')
const sortAlphabetically = ({ pkgid: a }, { pkgid: b }) => localeCompare(a, b)

const humanOutput = ({ color, result, seenItems, unicode }) => {
// we need to traverse the entire tree in order to determine which items
Expand Down
3 changes: 2 additions & 1 deletion lib/outdated.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const color = require('chalk')
const styles = require('ansistyles')
const npa = require('npm-package-arg')
const pickManifest = require('npm-pick-manifest')
const localeCompare = require('@isaacs/string-locale-compare')('en')

const Arborist = require('@npmcli/arborist')

Expand Down Expand Up @@ -85,7 +86,7 @@ class Outdated extends ArboristWorkspaceCmd {
}))

// sorts list alphabetically
const outdated = this.list.sort((a, b) => a.name.localeCompare(b.name, 'en'))
const outdated = this.list.sort((a, b) => localeCompare(a.name, b.name))

if (outdated.length > 0)
process.exitCode = 1
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/completion/installed-deep.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const { resolve } = require('path')
const Arborist = require('@npmcli/arborist')
const localeCompare = require('@isaacs/string-locale-compare')('en')

const installedDeep = async (npm) => {
const {
Expand All @@ -15,8 +16,7 @@ const installedDeep = async (npm) => {
return i
})
.filter(i => (i.depth - 1) <= depth)
.sort((a, b) => a.depth - b.depth)
.sort((a, b) => a.depth === b.depth ? a.name.localeCompare(b.name, 'en') : 0)
.sort((a, b) => (a.depth - b.depth) || localeCompare(a.name, b.name))
Copy link
Member

Choose a reason for hiding this comment

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

👍


const res = new Set()
const gArb = new Arborist({ global: true, path: resolve(npm.globalDir, '..') })
Expand Down
3 changes: 2 additions & 1 deletion lib/utils/config/describe-all.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const definitions = require('./definitions.js')
const localeCompare = require('@isaacs/string-locale-compare')('en')
const describeAll = () => {
// sort not-deprecated ones to the top
/* istanbul ignore next - typically already sorted in the definitions file,
Expand All @@ -7,7 +8,7 @@ const describeAll = () => {
const sort = ([keya, {deprecated: depa}], [keyb, {deprecated: depb}]) => {
return depa && !depb ? 1
: !depa && depb ? -1
: keya.localeCompare(keyb, 'en')
: localeCompare(keya, keyb)
}
return Object.entries(definitions).sort(sort)
.map(([key, def]) => def.describe())
Expand Down
3 changes: 2 additions & 1 deletion lib/utils/npm-usage.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const { dirname } = require('path')
const { cmdList } = require('./cmd-list')
const localeCompare = require('@isaacs/string-locale-compare')('en')

module.exports = (npm) => {
const usesBrowser = npm.config.get('viewer') === 'browser'
Expand Down Expand Up @@ -62,7 +63,7 @@ const usages = (npm) => {
maxLen = Math.max(maxLen, c.length)
return set
}, [])
.sort((a, b) => a[0].localeCompare(b[0], 'en'))
.sort(([a], [b]) => localeCompare(a, b))
.map(([c, usage]) => `\n ${c}${' '.repeat(maxLen - c.length + 1)}${
(usage.split('\n').join('\n' + ' '.repeat(maxLen + 5)))}`)
.join('\n')
Expand Down
11 changes: 5 additions & 6 deletions lib/utils/tar.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ const ssri = require('ssri')
const npmlog = require('npmlog')
const formatBytes = require('./format-bytes.js')
const columnify = require('columnify')
const localeCompare = require('@isaacs/string-locale-compare')('en', {
sensitivity: 'case',
numeric: true,
})

const logTar = (tarball, opts = {}) => {
const { unicode = false, log = npmlog } = opts
Expand Down Expand Up @@ -75,12 +79,7 @@ const getContents = async (manifest, tarball) => {
algorithms: ['sha1', 'sha512'],
})

const comparator = (a, b) => {
return a.path.localeCompare(b.path, 'en', {
sensitivity: 'case',
numeric: true,
})
}
const comparator = ({ path: a }, { path: b }) => localeCompare(a, b)

const isUpper = (str) => {
const ch = str.charAt(0)
Expand Down
36 changes: 28 additions & 8 deletions node_modules/@isaacs/string-locale-compare/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion node_modules/@isaacs/string-locale-compare/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 7 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"./package.json": "./package.json"
},
"dependencies": {
"@isaacs/string-locale-compare": "^1.1.0",
"@npmcli/arborist": "^2.9.0",
"@npmcli/ci-detect": "^1.2.0",
"@npmcli/config": "^2.3.0",
Expand Down