Skip to content

Commit 5e0eb36

Browse files
lib: improve comments
1 parent f65791d commit 5e0eb36

File tree

1 file changed

+34
-19
lines changed

1 file changed

+34
-19
lines changed

lib/find-python.js

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,9 @@ class PythonFinder {
288288
)}" to get Python executable path`
289289
)
290290

291-
const result = await check.checkFunc.apply(this, [check ? check.arg : null])
291+
const result = await check.checkFunc.apply(this, [
292+
check ? check.arg : null
293+
])
292294
fail = false
293295
this.succeed(result.path, result.version)
294296

@@ -324,7 +326,10 @@ class PythonFinder {
324326
}
325327

326328
return new Promise((resolve, reject) => {
327-
this.run(exec, args, shell).then(this.checkExecPath).then(resolve).catch(reject)
329+
this.run(exec, args, shell)
330+
.then(this.checkExecPath)
331+
.then(resolve)
332+
.catch(reject)
328333
})
329334
}
330335

@@ -361,6 +366,7 @@ class PythonFinder {
361366
// to pass both path and version
362367
return new Promise((resolve, reject) => {
363368
this.log.verbose(`- executing "${execPath}" to get version`)
369+
364370
this.run(execPath, this.argsVersion, false)
365371
.then((ver) => {
366372
// ? may be better code for version check
@@ -392,9 +398,11 @@ class PythonFinder {
392398
colorizeOutput(RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED')
393399
)
394400
// object with error passed for conveniences
395-
// (becouse we can also pass stderr or some additional staff)
401+
// (becouse we may want to pass also stderr or some additional stuff)
396402
// eslint-disable-next-line prefer-promise-reject-errors
397-
reject({ err: new Error(`Found unsupported Python version ${ver}`) })
403+
reject({
404+
err: new Error(`Found unsupported Python version ${ver}`)
405+
})
398406
}
399407

400408
resolve({ path: execPath, version: ver })
@@ -422,6 +430,7 @@ class PythonFinder {
422430
function (resolve, reject) {
423431
const env = extend({}, this.env)
424432
env.TERM = 'dumb'
433+
/** @type cp.ExecFileOptionsWithStringEncoding */
425434
const opts = { env: env, shell: shell }
426435

427436
this.log.verbose(
@@ -434,24 +443,27 @@ class PythonFinder {
434443
)
435444
this.log.silly('execFile: opts = ', JSON.stringify(opts, null, 2))
436445

437-
//* assume that user use utf8 compatible termnal
438-
439-
//* prosible outcomes with error messages (err.message, error.stack, stderr)
440-
//! on windows:
446+
//* prosible outcomes with error messages on windows (err.message, error.stack?, stderr)
441447
// issue of encoding (garbage in terminal ) when 866 or any other locale code
442448
// page is setted
443449
// possible solutions:
444-
// use "cmd" command with flag "/U" and "/C" (for more informatiom help cmd)
450+
// 1. let it be as it is and just warn user that it should use utf8
451+
// (already done in this.catchErros's info statement)
452+
// 2. somehow determine user's terminal encdoing and use utils.TextDecoder
453+
// with getting raw buffer from execFile.
454+
// Requires to correct error.message becouse garbage persists there
455+
// 3. Forse user's terminal to use utf8 encoding via e.g. run "chcp 65001". May broke some user's programs
456+
// 4. use "cmd" command with flag "/U" and "/C" (for more informatiom run "help cmd")
445457
// which "Causes the output of
446-
// internal commands to a pipe or file to be Unicode" (utf16le)
447-
//* note: find-python-script.py send output in utf8 then may become necessary
448-
//* to reencoded string with Buffer.from(stderr).toString() or something
458+
// internal commands ... to be Unicode" (utf16le)
459+
//* note: find-python-script.py already send output in utf8 then may become necessary
460+
//* to reencode string with Buffer.from(stderr).toString() or something
449461
//* similar (if needed)
450-
// for this solution all args should be passed as SINGLE string in quotes
451-
// becouse cmd has such signature: CMD [/A | /U] [/Q] [/D] [/E:ON | /E:OFF]
452-
// [/F:ON | /F:OFF] [/V:ON | /V:OFF] [[/S] [/C | /K] string]
462+
// for this solution all execFile call should look like execFile("cmd /U /C", [command to run, arg1, arg2, ...])
453463
//* all pathes/commands and each argument must be in quotes becouse if they have
454-
//* spaces they will broke everything
464+
//* spaces inside they will broke everything
465+
466+
//* assume that user use utf8 compatible terminal
455467
this.execFile(exec, args, opts, execFileCallback.bind(this))
456468

457469
/**
@@ -470,7 +482,7 @@ class PythonFinder {
470482
if (err || stderr) {
471483
reject({ err: err || null, stderr: stderr || null })
472484
} else {
473-
// trim function removing endings which couse bugs when comparing strings
485+
// trim() function remove string endings that break string comparsion
474486
const stdoutTrimed = stdout.trim()
475487
resolve(stdoutTrimed)
476488
}
@@ -526,7 +538,9 @@ class PythonFinder {
526538
this.log.silly(err ? err.stack : '')
527539

528540
if (stderr) {
529-
this.addLog(`${colorizeOutput(RED, 'STDERR:')} ${stderr ? stderr.trim() : ''}`)
541+
this.addLog(
542+
`${colorizeOutput(RED, 'STDERR:')} ${stderr ? stderr.trim() : ''}`
543+
)
530544
}
531545
}
532546
this.addLog('--------------------------------------------')
@@ -563,7 +577,8 @@ class PythonFinder {
563577
// X
564578
const info = [
565579
'**********************************************************',
566-
'If you have non-displayed characters set "UTF-8" encoding.',
580+
'If you have non-displayed characters, please set "UTF-8"',
581+
'encoding.',
567582
'You need to install the latest version of Python.',
568583
'Node-gyp should be able to find and use Python. If not,',
569584
'you can try one of the following options:',

0 commit comments

Comments
 (0)