errors,tools: ASCIIbetical instead of alphabetical

[refack]

Fixes: #15576

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

errors,tools

[refack]

/cc @maclover7

[lpinca]

typo: pref -> prev

[refack]

I reworked the error message completely:

d:\code\node-cur\lib\internal\errors.js
  147:1  error  Out of ASCIIbetical order - ERR_HTTP_TRAILER_INVALID >= ERR_HTTP2_CONNECT_AUTHORITY           alphabetize-errors
  147:1  error                                  (95)_                >=         2(50)                         alphabetize-errors
  165:1  error  Out of ASCIIbetical order - ERR_HTTP2_HEADER_SINGLE_VALUE >= ERR_HTTP2_HEADERS_AFTER_RESPOND  alphabetize-errors
  165:1  error                                          (95)_             >=                 S(83)            alphabetize-errors
  262:1  error  Out of ASCIIbetical order - ERR_OUT_OF_RANGE >= ERR_OUTOFMEMORY                               alphabetize-errors
  262:1  error                                 (95)_         >=        O(79)                                  alphabetize-errors

✖ 6 problems (6 errors, 0 warnings)

[lpinca]

Isn't it easier to just show the index where the two strings differ?

[BridgeAR]

This is not really readable. Especially the variable names are pretty bad. Without checking to closely I am also pretty sure this could probably be simplified a bit so it is better to read afterwards.

[refack]

Easy? 😤 easy for someone else...
_{yeah I was bored this morning}

But seriously, I think it's a nice and informative feedback, and I don't assume this error should happen too often, so I can take it or leave it..

[lpinca]

My point is that I don't care if _ >= O for example. I find it sufficient to know the index where the issue happens (e.g. 7 for ERR_OUT_OF_RANGE and ERR_OUTOFMEMORY) but I understand this is very personal.

[refack]

Ack.

[maclover7]

@refack interesting PR! Overall looks good, only concern is that the new "is there a problem" / message generating logic is a little bit hard to grok -- would it be possible to move these back to separate functions?

[refack]

message generating logic is a little bit hard to grok

Yeah a little bit overboard. Not sure if I could understand it in a week...

[refack]

Removed obfus code. Now the errors looks like:

d:\code\node-cur\lib\internal\errors.js
  147:1  error  Out of ASCIIbetical order - ERR_HTTP_TRAILER_INVALID >= ERR_HTTP2_CONNECT_AUTHORITY           alphabetize-errors
  165:1  error  Out of ASCIIbetical order - ERR_HTTP2_HEADER_SINGLE_VALUE >= ERR_HTTP2_HEADERS_AFTER_RESPOND  alphabetize-errors
  262:1  error  Out of ASCIIbetical order - ERR_OUT_OF_RANGE >= ERR_OUTOFMEMORY                               alphabetize-errors

✖ 3 problems (3 errors, 0 warnings)

and IMHO it's informative enough.

Linter CI: https://ci.nodejs.org/job/node-test-linter/12025/

[maclover7]

I'm not a collaborator, but 👍, much simpler now

[jasnell]

I see the point but I'm not a fan of landing this right now. Perhaps after we get all of the codes migrated, otherwise it just ends up causing additional churn.

[refack]

I see the point but I'm not a fan of landing this right now. Perhaps after we get all of the codes migrated, otherwise it just ends up causing additional churn.

Other way around. It's already in and has been breaking the lint CI for over a week. I had to hack the lint machine to use LC_ALL=en_US.

[refack]

@jasnell linter fails when it runs on the other (unpatched) machine test-rackspace-freebsd10-x64-1 https://ci.nodejs.org/job/node-test-linter/12073/

[jasnell]

hmm... ok. I'd just generally prefer that we not shuffle these ids around too much more until we get them all migrated as it makes getting those PRs landed much more difficult.

Removing my -1

[refack]

@nodejs/release you might want to cherry-pick to v8.x-staging (https://ci.nodejs.org/job/node-test-linter/12101/)

not ok 3 - /usr/home/iojs/build/workspace/node-test-linter/lib/internal/errors.js
  ---
  message: Errors in lib/internal/errors.js must be alphabetized
  severity: error
  data:
    line: 244
    column: 1
    ruleId: alphabetize-errors
  messages:
    - message: Errors in lib/internal/errors.js must be alphabetized
      severity: error
      data:
        line: 251
        column: 1
        ruleId: alphabetize-errors
  ...

[MylesBorins]

I lied... need it one more time

please open a new PR