Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REPL special commands need arg validation #52218

Closed
MrJithil opened this issue Mar 26, 2024 · 1 comment · Fixed by #52225
Closed

REPL special commands need arg validation #52218

MrJithil opened this issue Mar 26, 2024 · 1 comment · Fixed by #52225
Labels
good first issue Issues that are suitable for first-time contributors.

Comments

@MrJithil
Copy link
Member

MrJithil commented Mar 26, 2024

In the current implementation, the REPL commands such as .save and .load accept a filePath as arguments.

It would greatly improve user experience if we could validate these arguments to prevent attempts to save or load files with empty filenames.

In case the file is an empty string, we should print the error message, show the prompt to the user and return.

ERR_MISSING_ARGS would be a good fit here to frame the message.
Eg: const { message } = new ERR_MISSING_ARGS('file');

String values like 0, false, null, undefined etc are still a valid filenames and good to write some unit tests for this.

@MrJithil MrJithil added the good first issue Issues that are suitable for first-time contributors. label Mar 26, 2024
@thomas-mauran
Copy link
Contributor

Hello @MrJithil, I am interested in working on this issue. Could you guide me on where to start?

thomas-mauran added a commit to thomas-mauran/node that referenced this issue Mar 26, 2024
This commit adds a proper error message using ERR_MISSING_ARGS('file')
when a .save or .load REPL command is runned. This commit also adds
test for both of this cases.

Fixes: nodejs#52218

Signed-off-by: Thomas Mauran <[email protected]>
nodejs-github-bot pushed a commit that referenced this issue Apr 6, 2024
This commit adds a proper error message using ERR_MISSING_ARGS('file')
when a .save or .load REPL command is runned. This commit also adds
test for both of this cases.

Fixes: #52218

Signed-off-by: Thomas Mauran <[email protected]>
PR-URL: #52225
Reviewed-By: Kohei Ueno <[email protected]>
marco-ippolito pushed a commit that referenced this issue May 2, 2024
This commit adds a proper error message using ERR_MISSING_ARGS('file')
when a .save or .load REPL command is runned. This commit also adds
test for both of this cases.

Fixes: #52218

Signed-off-by: Thomas Mauran <[email protected]>
PR-URL: #52225
Reviewed-By: Kohei Ueno <[email protected]>
marco-ippolito pushed a commit that referenced this issue May 3, 2024
This commit adds a proper error message using ERR_MISSING_ARGS('file')
when a .save or .load REPL command is runned. This commit also adds
test for both of this cases.

Fixes: #52218

Signed-off-by: Thomas Mauran <[email protected]>
PR-URL: #52225
Reviewed-By: Kohei Ueno <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants