Skip to content

Commit 0f4097a

Browse files
lib: improve comments
1 parent 0cce679 commit 0f4097a

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
@@ -316,7 +316,9 @@ class PythonFinder {
316316
)}" to get Python executable path`
317317
)
318318

319-
const result = await check.checkFunc.apply(this, [check ? check.arg : null])
319+
const result = await check.checkFunc.apply(this, [
320+
check ? check.arg : null
321+
])
320322
fail = false
321323
this.succeed(result.path, result.version)
322324

@@ -352,7 +354,10 @@ class PythonFinder {
352354
}
353355

354356
return new Promise((resolve, reject) => {
355-
this.run(exec, args, shell).then(this.checkExecPath).then(resolve).catch(reject)
357+
this.run(exec, args, shell)
358+
.then(this.checkExecPath)
359+
.then(resolve)
360+
.catch(reject)
356361
})
357362
}
358363

@@ -389,6 +394,7 @@ class PythonFinder {
389394
// to pass both path and version
390395
return new Promise((resolve, reject) => {
391396
this.log.verbose(`- executing "${execPath}" to get version`)
397+
392398
this.run(execPath, this.argsVersion, false)
393399
.then((ver) => {
394400
// ? may be better code for version check
@@ -420,9 +426,11 @@ class PythonFinder {
420426
colorizeOutput(RED, 'THIS VERSION OF PYTHON IS NOT SUPPORTED')
421427
)
422428
// object with error passed for conveniences
423-
// (becouse we can also pass stderr or some additional staff)
429+
// (becouse we may want to pass also stderr or some additional stuff)
424430
// eslint-disable-next-line prefer-promise-reject-errors
425-
reject({ err: new Error(`Found unsupported Python version ${ver}`) })
431+
reject({
432+
err: new Error(`Found unsupported Python version ${ver}`)
433+
})
426434
}
427435

428436
resolve({ path: execPath, version: ver })
@@ -450,6 +458,7 @@ class PythonFinder {
450458
function (resolve, reject) {
451459
const env = extend({}, this.env)
452460
env.TERM = 'dumb'
461+
/** @type cp.ExecFileOptionsWithStringEncoding */
453462
const opts = { env: env, shell: shell }
454463

455464
this.log.verbose(
@@ -462,24 +471,27 @@ class PythonFinder {
462471
)
463472
this.log.silly('execFile: opts = ', JSON.stringify(opts, null, 2))
464473

465-
//* assume that user use utf8 compatible termnal
466-
467-
//* prosible outcomes with error messages (err.message, error.stack, stderr)
468-
//! on windows:
474+
//* prosible outcomes with error messages on windows (err.message, error.stack?, stderr)
469475
// issue of encoding (garbage in terminal ) when 866 or any other locale code
470476
// page is setted
471477
// possible solutions:
472-
// use "cmd" command with flag "/U" and "/C" (for more informatiom help cmd)
478+
// 1. let it be as it is and just warn user that it should use utf8
479+
// (already done in this.catchErros's info statement)
480+
// 2. somehow determine user's terminal encdoing and use utils.TextDecoder
481+
// with getting raw buffer from execFile.
482+
// Requires to correct error.message becouse garbage persists there
483+
// 3. Forse user's terminal to use utf8 encoding via e.g. run "chcp 65001". May broke some user's programs
484+
// 4. use "cmd" command with flag "/U" and "/C" (for more informatiom run "help cmd")
473485
// which "Causes the output of
474-
// internal commands to a pipe or file to be Unicode" (utf16le)
475-
//* note: find-python-script.py send output in utf8 then may become necessary
476-
//* to reencoded string with Buffer.from(stderr).toString() or something
486+
// internal commands ... to be Unicode" (utf16le)
487+
//* note: find-python-script.py already send output in utf8 then may become necessary
488+
//* to reencode string with Buffer.from(stderr).toString() or something
477489
//* similar (if needed)
478-
// for this solution all args should be passed as SINGLE string in quotes
479-
// becouse cmd has such signature: CMD [/A | /U] [/Q] [/D] [/E:ON | /E:OFF]
480-
// [/F:ON | /F:OFF] [/V:ON | /V:OFF] [[/S] [/C | /K] string]
490+
// for this solution all execFile call should look like execFile("cmd /U /C", [command to run, arg1, arg2, ...])
481491
//* all pathes/commands and each argument must be in quotes becouse if they have
482-
//* spaces they will broke everything
492+
//* spaces inside they will broke everything
493+
494+
//* assume that user use utf8 compatible terminal
483495
this.execFile(exec, args, opts, execFileCallback.bind(this))
484496

485497
/**
@@ -498,7 +510,7 @@ class PythonFinder {
498510
if (err || stderr) {
499511
reject({ err: err || null, stderr: stderr || null })
500512
} else {
501-
// trim function removing endings which couse bugs when comparing strings
513+
// trim() function remove string endings that break string comparsion
502514
const stdoutTrimed = stdout.trim()
503515
resolve(stdoutTrimed)
504516
}
@@ -554,7 +566,9 @@ class PythonFinder {
554566
this.log.silly(err ? err.stack : '')
555567

556568
if (stderr) {
557-
this.addLog(`${colorizeOutput(RED, 'STDERR:')} ${stderr ? stderr.trim() : ''}`)
569+
this.addLog(
570+
`${colorizeOutput(RED, 'STDERR:')} ${stderr ? stderr.trim() : ''}`
571+
)
558572
}
559573
}
560574
this.addLog('--------------------------------------------')
@@ -591,7 +605,8 @@ class PythonFinder {
591605
// X
592606
const info = [
593607
'**********************************************************',
594-
'If you have non-displayed characters set "UTF-8" encoding.',
608+
'If you have non-displayed characters, please set "UTF-8"',
609+
'encoding.',
595610
'You need to install the latest version of Python.',
596611
'Node-gyp should be able to find and use Python. If not,',
597612
'you can try one of the following options:',

0 commit comments

Comments
 (0)