Supporting passing a filename that contains a list of arguments as the last argument

[DanTup]

Allows the final argument to be @filename where filename is a path (relative to cwd) to a file that contains arguments to replace it before parsing.

This allows arguments longer than Windows supports to be provided (see Dart-Code/Dart-Code#5473).

cc @jakemac53

[jakemac53]

This should really be a replacement for any arguments, and not only test paths.

For instance this should work with an arbitrary number of -n arguments as well.

This does require some manual pre-processing of the input args as opposed to using rest but ultimately makes things more flexible.

[DanTup]

This should really be a replacement for any arguments, and not only test paths.

Oh, I see! Should we do this in the _Parser constructor (which means if we throw, it'll occur here and not during parse), or do them in parse (which means no code could try to access the internal _options before parse is called)?

[DanTup]

I guess a related question is whether this even belongs in Configuration.parse() or below, or should be done in executable.dart in _execute before calling Configuration.parse?

[jakemac53]

I guess a related question is whether this even belongs in Configuration.parse() or below, or should be done in executable.dart in _execute before calling Configuration.parse?

I would just do it in executable.dart - that is how we handle it most other places.

[DanTup]

@jakemac53 I've updated it, PTAL :-)

[DanTup]

@jakemac53 I was just starting to do something equivalent for flutter test and because of how the arg parsing works, it's going to be much easier to do it for all of flutter than flutter test (because it uses the CommandRunner from pkg:args and parses args before it gets into the Test command).

So now I'm wondering if it would make sense to do the same for dart (eg. support this for all dart commands including things like dart run rather than only test)? 🤔 I don't know if it would be as straight forwarded for dart, but the consistency might be nice. WDYT? (cc @bkonyi in case he has any opinions)

Edit: Flutter PR is at flutter/flutter#167272

[DanTup]

btw, do you need to do something additional to allow tests to run?

[jakemac53]

btw, do you need to do something additional to allow tests to run?

I think previously this was just the very first time you send a PR it would make a contributor approve things, and if you have any commits in the history it would run them without approval. Maybe that changed, not sure.

[github-actions]

PR Health

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

[jakemac53]

So now I'm wondering if it would make sense to do the same for dart (eg. support this for all dart commands including things like dart run rather than only test)? 🤔 I don't know if it would be as straight forwarded for dart, but the consistency might be nice. WDYT? (cc @bkonyi in case he has any opinions)

It sounds reasonable to me, but also I think you could land this here first if you wanted, and it shouldn't get in the way. If dart ... starts doing it then this just won't see the @fileName args any more.

[DanTup]

It sounds reasonable to me, but also I think you could land this here first if you wanted, and it shouldn't get in the way. If dart ... starts doing it then this just won't see the @filename args any more.

Yeah, good point. Let's continue with this as-is then because it'll likely be simpler to land and allow us to solve the immediate issue sooner.

The failed check was because I didn't add to test_core changelog. I've added there too but also left it in pkg:test since it seems relevant there, but let me know if you'd like me to change that.

(You may need to re-kick the bots it seems!)

[bkonyi]

This is definitely interesting. Do we have any concern that @ is a valid character in file names?

I like the idea of having general support for this in dart (and maybe flutter as well), but it'd be good to give this more thought before committing. For example, I can imagine wanting to support passing a long list of commonly set arguments from a file while also passing the script path as the last argument (i.e., dart run @my-args.txt foo.dart).

Would it be more discoverable and flexible if we added a global --args-from-file option instead?

[DanTup]

Do we have any concern that @ is a valid character in file names?

It's a possibility, though if this convention exists elsewhere, maybe it's not unreasonable to not support it. We could also restrict it in other ways (must be .txt for ex.), but that might feel a bit weird.

and maybe flutter as well

I started a PR at flutter/flutter#167272 for this for discussion too, and I do think it'd be good to have something consistent across the two.

For example, I can imagine wanting to support passing a long list of commonly set arguments from a file while also passing the script path as the last argument (i.e., dart run @my-args.txt foo.dart).

Supporting that would be quite trivial, it just increases the chances of a real arg wanting to start with @. We could pick something else, but if there's an existing convention of @ it might be nice to use (I don't know how widespread that convention is though, Bazel was linked above, I don't know if it is implemented elsewhere).

Would it be more discoverable and flexible if we added a global --args-from-file option instead?

Perhaps, but I it could have drawbacks:

• You may have to parse the args twice - once to get that flag, and then again after substitution
• Knowing where to insert those args might make parsing more complex (if we use ArgParser, I don't think it tells us where the arg was, and if we don't, we might not handle things like quotes and --x=y vs --x y the same?)

Personally, I'm not too concerned with what the solution is, only that there is a solution - so I'm happy to go with whatever others feel would be best (and ideally that we can do consistently across tools).

[bkonyi]

I think it's worth having a discussion about this with a wider group (probably dart-cli@) before we commit to adding this generally, especially if we want to support additional trailing arguments.

[jakemac53]

Personally, I'm not too concerned with what the solution is, only that there is a solution - so I'm happy to go with whatever others feel would be best (and ideally that we can do consistently across tools).

Agreed, and there is precedent with at least bazel and our own tools which support bazel. These aren't really user facing though, but I don't ever expect any user to be doing this.

Do we have any concern that @ is a valid character in file names?

I am not concerned about this personally, pretty extreme edge case imo. You would have to be trying to run tests which both live at the root of your repo, and have a name starting with @. That is probably zero people. And if its not, I am personally willing to just tell those people that our package does not work with that pattern.

[DanTup]

@bkonyi

I think it's worth having a discussion about this with a wider group (probably dart-cli@) before we commit to adding this generally, especially if we want to support additional trailing arguments.

Is this an internal group? Should I file an SDK issue on GH?

I am not concerned about this personally, pretty extreme edge case imo. You would have to be trying to run tests which both live at the root of your repo, and have a name starting with @. That is probably zero people. And if its not, I am personally willing to just tell those people that our package does not work with that pattern.

I'd also be ok with that, but it would also be nice that if we landed something in pkg:test in the short-term with the goal of supporting it in dart ... later, that we don't change it in a way that could break changes shipped in Dart-Code, so it would be good to get some buy-in of that for dart and flutter before committing.

[bkonyi]

Is this an internal group? Should I file an SDK issue on GH?

Yeah, it's an internal group. Filing an SDK issue with the proposed behavior would be great!

[DanTup]

@bkonyi I've opened dart-lang/sdk#60551. If you're able to attract the right peoples eyes, that would be appreciated (it would be good to agree a format and answer the questions so even if we choose to just ship something in pkg:test initially, we can reduce the chance of needing to make any breaking changes later).