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

Drop Option wrapper on Manifest target vecs #5336

Closed
wants to merge 2 commits into from
Closed

Drop Option wrapper on Manifest target vecs #5336

wants to merge 2 commits into from

Conversation

dwijnand
Copy link
Member

As opposed to other manifest keys, such as authors, where a difference
may be drawn between:

authors = []

and the complete absence of the authors key the same cannot be said
for the target keys which are TOML Array of Tables types for which
the only way to define an empty array is by never defining any element
in it.

As opposed to other manifest keys, such as `authors`, where a difference
may be drawn between:

    authors = []

and the complete absence of the `authors` key the same cannot be said
for the target keys which are TOML [Array of Tables][1] types for which
the only way to define an empty array is by never defining any element
in it.

[1]: https://github.com/toml-lang/toml#array-of-tables
@rust-highfive
Copy link

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

@dwijnand
Copy link
Member Author

this will clash with my #5335, but that's fine, I was looking to see if it was possible to simplify

let at_least_a_single_target_is_listed = toml_targets
	.map(|targets| !targets.is_empty())
    .unwrap_or(false);

to just

let at_least_a_single_target_is_listed = !toml_targets.is_empty();

feedback very welcome, I'm happy to iterate (and learn).

was this required by the Option? is & the better practice? feedback welcome
@matklad
Copy link
Member

matklad commented Apr 10, 2018

This is a horrible edge-case, but unfortunately one can do

test = []

[package]
name = "foo"
version = "0.1.0"
authors = ["[email protected]"]

[dependencies]

@dwijnand
Copy link
Member Author

Ah, I see what you mean. And now I understand better where the TOML docs explained that.

Shame.

@dwijnand dwijnand closed this Apr 10, 2018
@dwijnand dwijnand deleted the non-opt-target-vecs branch April 10, 2018 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants