-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Refactor CLI entry point #2157
Refactor CLI entry point #2157
Conversation
6e2f8d9
to
172d927
Compare
}).map_err(|(err, _worker)| print_err_and_exit(err)) | ||
}); | ||
tokio_util::run(main_future); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ry PTAL at it. It seem that execute_mod_async
does not evaluate and run file if prefetch
is set to true, so simple letting worker finish the job seems to work fine. I'm not sure though
c51aed8
to
691aea7
Compare
691aea7
to
9d8c7d3
Compare
51c43e5
to
80ef15d
Compare
} | ||
); | ||
} | ||
fn flags_from_vec(args: Vec<String>) -> DenoFlags { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this function should be the high level public “parse_flags” while the other function which is currently called “parse_flags” should be either combined into this or called something else (and be internal to this module)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I had similar feeling about it, but that means we no longer have matches
var available in main()
, that gives nice switch on matches.subcommand()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively parse_flags
may return DenoFlags, ArgMatches
to handle both cases
The manual.md file also seems to need to be modified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
very nice clean up - thanks !
PS we should add a "deno help" command - it feels like that would be expected now. |
@ry no problem, I can add it in an hour or so. I understand you want to add |
This PR refactors
main.rs
to use dedicated functions for differentdeno <subcommand>
, as discussed in #2084EDIT: This PR should be blocked by #2113
EDIT2: As suggested by @kevinkassimo in #2102 and changed
--types
and--prefetch
to respective subcommands: