feat: support various encoding #446
Conversation
| @@ -6,6 +6,8 @@ import { quote } from 'shell-quote'; | |||
| import _ from 'lodash'; | |||
| import { formatEnoent } from './helpers'; | |||
| import { createInterface } from 'node:readline'; | |||
| import iconv from 'iconv-lite'; | |||
There was a problem hiding this comment.
prefer to use import { decode } from 'iconv-lite'; syntax
| if (this.proc.stdout) { | ||
| this.proc.stdout.on('data', (chunk) => handleOutput({stdout: chunk.toString(), stderr: ''})); | ||
| this.proc.stdout.on('data', (chunk) => handleOutput({stdout: decode(chunk, encoding), stderr: ''})); |
There was a problem hiding this comment.
should it only be applied to Windows?
There was a problem hiding this comment.
also, I assume it only makes sense to apply decode if the requested encoding differs from the default one (e.g. utf8)
| @@ -48,6 +48,7 @@ | |||
| }, | |||
| "dependencies": { | |||
| "bluebird": "^3.7.2", | |||
| "iconv-lite": "0.6.3", | |||
There was a problem hiding this comment.
please use version range instead, e.g. ^0.x
There was a problem hiding this comment.
I assume this project has not been regularly maintained recently. Have you considered using some alternatives? For example, the Buffer.toString method also supports setting encoding as an argument
There was a problem hiding this comment.
How about the following solution? It provides the buffer as output, similar to exec.
#447
There was a problem hiding this comment.
yes, the other solution adds more flexibility
|
I replace this PR with #447. |
The Windows operating system often has a unique default encoding set for the terminal (such as PowerShell) depending on the country. For example, when running WinAppDriver as a
SubProcess, if an OS error occurs in a non-English-speaking country, the error message will be translated into the local language, making it unreadable. This issue arises because the encoding options provided by Node.js are limited and cannot resolve the problem.I propose using
iconv-liteas a solution.