Skip to content

Conversation

@CoderDake
Copy link
Contributor

The devtools server is getting a --dtd-uri flag in https://dart-review.googlesource.com/c/sdk/+/346922

This change allows devtools_tool serve to pass along that flag.

@CoderDake CoderDake marked this pull request as ready for review February 9, 2024 18:47
@CoderDake CoderDake requested a review from a team as a code owner February 9, 2024 18:47
kenzieschmoll
kenzieschmoll previously approved these changes Feb 9, 2024
@kenzieschmoll
Copy link
Member

Actually wait a minute. Since we are already forwarding remaining arguments through to the command to start the DevTools server, I don't think we need to parse this manually from the serve command. Try passing the --dtd-uri flag to the serve command without this change.

@kenzieschmoll kenzieschmoll self-requested a review February 9, 2024 18:56
@kenzieschmoll kenzieschmoll dismissed their stale review February 9, 2024 18:56

undo approval

@CoderDake
Copy link
Contributor Author

@kenzieschmoll without this change i got the following error:

Screenshot 2024-02-09 at 1 59 57 PM

So I figured that adding it here was kind of like allowlisting it

@DanTup
Copy link
Contributor

DanTup commented Feb 14, 2024

I think we do need this for the same reasons we have things like --allow-embed and --machine. The current arg parser allows for additional arguments, but not additional options/flags (unless we use AllowAnythingParser). (see discussion in #6670)

Copy link
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

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

LGTM. Can you add a TODO to consider using AllowAnythingParser? Would also be good to separate the flags consts that are there to pass through to the DevTools server, and add a comment. (_dtdUriFlag, _machineFlag, _allowEmbeddingFlag) - any flag that we aren't removing later is intended to be passed through.

@CoderDake CoderDake merged commit 670bd5e into flutter:master Feb 15, 2024
@CoderDake CoderDake deleted the dtd-uri-flag branch February 15, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants