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

Serialize SandboxMode enum to string for JSON #4041

Merged
merged 1 commit into from Sep 22, 2020

Conversation

cole-h
Copy link
Member

@cole-h cole-h commented Sep 19, 2020

Rather than showing an integer as the default, instead show the string
referenced in the description.

The nix.conf.5 manpage used to show "default: 0", which is unnecessarily
opaque and confusing (doesn't 0 mean false, even though the default is
true?); now it properly shows that the default is true.

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Good idea. Are there any back compat issues we need to worry about?

@cole-h
Copy link
Member Author

cole-h commented Sep 19, 2020

I wouldn't think so. AFAICT this only affects the output of nix show-config --json (which displays the description and is what's used to generate the manpage) -- ./inst/bin/nix show-config --json --experimental-features nix-command | jq '.["sandbox"].defaultValue' now shows "true" rather than 0.

I also thought about making "true" an actual boolean true (same with "false") -- thoughts?

@Ericson2314
Copy link
Member

Ah great, so this is all --experimental-features nix-command stuff. Issue averted!

I also thought about making "true" an actual boolean true (same with "false") -- thoughts?

Sounds good too!

@cole-h cole-h force-pushed the enum-stringify branch 2 times, most recently from 414861b to eab7a97 Compare September 20, 2020 05:26
@cole-h
Copy link
Member Author

cole-h commented Sep 20, 2020

OK, done :)

(Maybe this will help people who parse the output from having to convert from a string to a boolean.)

src/libstore/globals.hh Outdated Show resolved Hide resolved
Rather than showing an integer as the default, instead show the boolean
referenced in the description.

The nix.conf.5 manpage used to show "default: 0", which is unnecessarily
opaque and confusing (doesn't 0 mean false, even though the default is
true?); now it properly shows that the default is true.
@edolstra edolstra merged commit 7dd8baa into NixOS:master Sep 22, 2020
@cole-h cole-h deleted the enum-stringify branch September 22, 2020 09:11
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.

None yet

3 participants