fix(cli): make --eval-file flag work without equals sign #31151
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR removes the require_equals(true) constraint from the --eval-file flag in the REPL subcommand to enable more flexible usage patterns, particularly for hashbang scripts. The change allows users to specify eval files using both --eval-file=path and --eval-file path syntax.
- Removed
.require_equals(true)from the--eval-fileflag definition - Added test coverage for space-separated syntax:
--eval-file ./script.ts - Added test coverage for multiple flag usage:
--eval-file ./a.ts --eval-file ./b.ts
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Removes the `require_equals(true)` constraint from the `--eval-file` flag in the REPL subcommand, allowing it to be used in hashbang contexts where the equals sign cannot be provided. This change enables the following usage patterns: - `deno repl --eval-file script.ts` (new) - `deno repl --eval-file=script.ts` (still works) - `deno repl --eval-file 1.ts --eval-file 2.ts` (multiple files) Also adds test coverage for: - Using --eval-file without equals sign - Multiple --eval-file flags Fixes denoland#30958
gn00295120
force-pushed
the
fix-eval-file-flag-30958
branch
from
ce757ba to
7076fa5
Compare
This addresses the Copilot PR review feedback that removing require_equals(true) would break the comma-separated syntax in the existing test at line 8782. Adding use_value_delimiter(true) maintains backward compatibility with: - Comma-separated values: --eval-file=./a.js,./b.ts,./c.ts - Space-separated syntax: --eval-file ./script.ts - Multiple flags: --eval-file ./a.ts --eval-file ./b.ts This change aligns with other flags in the codebase that support multiple values (allow-net, allow-env, etc.).
Format vec! declarations to single line as per dprint requirements.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #30958 by removing the
require_equals(true)constraint from the--eval-fileflag, making the equals sign optional.Problem
The
--eval-fileflag currently requires an equals sign (--eval-file=file.ts), which makes it impossible to use in hashbang contexts where arguments cannot contain equals signs.Solution
Removed
.require_equals(true)from the flag definition incli/args/flags.rs, allowing both syntaxes:deno repl --eval-file script.ts(new, works with hashbangs)deno repl --eval-file=script.ts(still works, backwards compatible)deno repl --eval-file 1.ts --eval-file 2.ts(multiple files)Changes
require_equals(true)constraint from--eval-fileflag--eval-fileflagsTesting
Added two new test cases:
repl_with_eval_file_flag_no_equals- Tests--eval-file script.tssyntaxrepl_with_eval_file_flag_multiple- Tests multiple flagsThe existing test
repl_with_eval_file_flagcontinues to work, ensuring backwards compatibility.Use Case
This enables the following hashbang usage: