readline: rename input field and remove excess params

[rexagod]

Removed excess parameters from the createInterface method,
renamed input field to options, in accordance with the docs,
and made corresponding changes in tests.

Refs: #31603 (comment)

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[BridgeAR]

CITGM https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2299/ (queued)

[jasnell]

If I'm reading this correctly is completely removes the ability to pass in the multiple arguments and changes the API. We can't do it like this. While the createInterface(input, output, completer, terminal) signature does not appear to be documented, it works and has not been deprecated. It would need to go through a proper runtime deprecation cycle before we can remove it.

[rexagod]

How can I initiate a "runtime deprecation cycle"? IMO, I don't see any reason to not incorporate this just because the older one "works". If anything I think it makes sense to try and adhere the code to the docs as much as possible and this should be acceptable on those terms. We could pass warnings for multiple param usage of readline in a patch version and implement this change in a major, maybe?

…

On Wed, 11 Mar 2020, 03:01 James M Snell, ***@***.***> wrote: ***@***.**** requested changes on this pull request. If I'm reading this correctly is completely removes the ability to pass in the multiple arguments and changes the API. We can't do it like this. While the createInterface(input, output, completer, terminal) signature does not appear to be documented, it works and has not been deprecated. It would need to go through a proper runtime deprecation cycle before we can remove it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#31991?email_source=notifications&email_token=AIAAUZ3O6GXWSPVGC44BSZDRG2WURA5CNFSM4K5E5HPKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCYYWQWQ#pullrequestreview-372336730>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIAAUZ53YHH44R5LHIOG7O3RG2WURANCNFSM4K5E5HPA> .

[addaleax]

@rexagod For runtime-deprecating something, you can look at usages of util.deprecate() in the JS source tree – basically, something that prints a deprecation warning is considered runtime-deprecated.

We could pass warnings for multiple param usage of readline in a patch version and implement this change in a major, maybe?

Runtime-deprecating/introducing warnings and removal are all semver-major changes.

And, while I understand that this is frustrating, I would also be -1 on introducing such a warning without a good reason – I don’t consider it being undocumented a good reason, personally.

[rexagod]

Thank you for the explanation @addaleax! There is however a deprecation (DEP0094) with similar cause to this one, where multiple params were reduced to one, but I guess that was under a different scenario.

Please feel free to close this, or let me know if you have any other plans regarding this issue.

[rexagod]

Hello, @addaleax @jasnell. I was wondering since this isn't closed yet, are there any plans on deprecating this?

[jasnell]

This PR cannot land in it's current state. It should either be changed to a deprecation or closed with a separate PR deprecating first. If you'd like to do that, please do :-)

[rexagod]

@jasnell The DeprecationWarning is causing 10 repl tests to fail. Running them with --no-deprecation flag seems to fix 3 of them, but for the rest, it seems to interfere with the core logic for what those tests were written for in the first place, and modifying them doesn't seem right.

Is there a way I can pass a --no-deprecation flag for a specific deprecation code?

[rexagod]

ping @jasnell

[jasnell]

Is there a way I can pass a --no-deprecation flag for a specific deprecation code?

Not currently

[rexagod]

Closing this for now. I'm focusing on getting my other not-stalled PRs merged at the moment. Will come back to this later if need be.