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

FAKE should fail on invalid input #1896

Closed
haf opened this issue Apr 29, 2018 · 6 comments
Closed

FAKE should fail on invalid input #1896

haf opened this issue Apr 29, 2018 · 6 comments

Comments

@haf
Copy link
Contributor

@haf haf commented Apr 29, 2018

E.g. ./build.sh --single-target --target Pack doesn't warn or anything; just proceeds with the default build.

@matthid

This comment has been minimized.

Copy link
Collaborator

@matthid matthid commented Apr 29, 2018

Accoding to the cli it is not really invalid input.

fake build --single-target --target Pack
Can be parsed as
fake.exe [fake_opts] build [build_opts] [--] [<scriptargs>...]
fake.exe [fake_opts] build [build_opts] [--] [<scriptargs>...]
So <scriptargs> is --single-target --target Pack

This in return will be parsed by the target module according to:
fake-run [target_opts] [target <target>] [--] [<targetargs>...]
fake-run [target_opts] [target <target>] [--] [<targetargs>...]

So <targetargs> is --single-target --target Pack and fake will run the default target with the two given arguments, according to https://fake.build/core-targets.html#Targets-with-arguments

Therefore this cannot be detected. We might trigger a warning if we can detect unparsed valid parameters. But the whole point of this is to allow conflicting parameters in a consistent way.

@matthid

This comment has been minimized.

Copy link
Collaborator

@matthid matthid commented Apr 29, 2018

We could also emit more logging about this process in verbose mode if that would help...

@matthid

This comment has been minimized.

Copy link
Collaborator

@matthid matthid commented Apr 29, 2018

Or one more idea: We could make this "argument" feature opt-in and report errors by default.
IE have Target.runOrDefault wouldn't support arguments and report an error message while Target.runOrDefaultWithArguments would.

@haf

This comment has been minimized.

Copy link
Contributor Author

@haf haf commented Apr 29, 2018

Why not require -- to pass arguments to the target?

Can you make it --single-target to be consistent with the rest of the world?

@matthid

This comment has been minimized.

Copy link
Collaborator

@matthid matthid commented Apr 29, 2018

Why not require -- to pass arguments to the target?

Because with the current system it will not work the way people would expect it to. In particular the first parser (from the runtime) the -- will be matched and then it will no longer be there for the second parser.

Can you make it --single-target to be consistent with the rest of the world?

Yes we could add / change that. Would we change --environmentvariable to --environment-variable as well?

@haf

This comment has been minimized.

Copy link
Contributor Author

@haf haf commented Apr 29, 2018

Yes those would be changed too.

matthid added a commit that referenced this issue May 1, 2018
@matthid matthid closed this in 910968f May 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.