Skip to content

Validate template w autosubmit#31

Open
lejeunerenard wants to merge 3 commits intoholepunchto:mainfrom
lejeunerenard:validate-template-w-autosubmit
Open

Validate template w autosubmit#31
lejeunerenard wants to merge 3 commits intoholepunchto:mainfrom
lejeunerenard:validate-template-w-autosubmit

Conversation

@lejeunerenard
Copy link

In the case that a default for a param is invalid by its own validation function, an Error will be thrown reporting the param's name and the msg.

Validation stops after the first validation that fails, prints to stderr and throws the same error. The error is thrown in addition to signal to the caller that something when wrong with running the interaction and the respond accordingly.

In the case that a default for a param is invalid by its own validation
function, an Error will be thrown reporting the param's `name` and the
`msg.
})

this._rl.input?.setMode(tty.constants.MODE_RAW)
if ('setMode' in this._rl.input) this._rl.input.setMode(tty.constants.MODE_RAW)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted this optional chaining to an explicit check for setMode() because the intent seems to set the mode if the stdin is a TTY. With it possibly being a Pipe instead, the optional chaining doesn't protect against calling setMode.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds default-value validation in the autosubmit flow and tests around failure and success cases

  • Introduces two new tests (autosubmit validation fails and autosubmit validation passes) in test/terminal.test.js
  • Updates #autosubmit in terminal.js to validate defaults, print to stderr, and throw on failure
  • Changes the raw‐mode guard for the readline input stream

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/terminal.test.js Added tests to verify autosubmit rejects invalid defaults and accepts valid ones
terminal.js Guarded setMode, made #autosubmit async with default validation and error reporting
Comments suppressed due to low confidence (3)

terminal.js:150

  • [nitpick] This private async method now encapsulates default validation logic; adding a brief JSDoc comment would clarify its purpose, parameters, and return value for future maintainers.
async #autosubmit () {

terminal.js:153

  • [nitpick] Shifting elements directly from this._params mutates the instance and prevents rerunning the prompt. Consider iterating over a shallow copy ([...this._params]) to preserve the original array.
while (this._params.length) {

terminal.js:162

  • [nitpick] Including a trailing newline in the thrown error message may be unexpected for callers. It might be cleaner to omit \n from the Error constructor and handle newlines only when writing to stderr.
throw Error(`Validating '${param.name}' parameter default failed: ${param.msg}\n`)

})

this._rl.input?.setMode(tty.constants.MODE_RAW)
if ('setMode' in this._rl.input) this._rl.input.setMode(tty.constants.MODE_RAW)
Copy link

Copilot AI Jun 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Guarding with in will throw if this._rl.input is null or undefined. Consider checking existence first, e.g. if (this._rl.input?.setMode) or if (this._rl.input && typeof this._rl.input.setMode === 'function') to avoid possible runtime errors.

Suggested change
if ('setMode' in this._rl.input) this._rl.input.setMode(tty.constants.MODE_RAW)
if (this._rl.input?.setMode) this._rl.input.setMode(tty.constants.MODE_RAW)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant