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

Consolidate bool string parsing into a common method #2811

Open
jimklimov opened this issue Feb 23, 2025 · 1 comment
Open

Consolidate bool string parsing into a common method #2811

jimklimov opened this issue Feb 23, 2025 · 1 comment
Labels
augeas Configuration file parser (reader, writer) multi-tool for scripting, etc. C++ C-bool Issues and PRs about C/C++ methods, headers and data types dealing with boolean types C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks enhancement refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings
Milestone

Comments

@jimklimov
Copy link
Member

jimklimov commented Feb 23, 2025

We have a number of locations in code (fewer than I expected, but still) that interpret boolean(-ish) strings from settings.

Some spots handle (case-insensitively) all of "true", "on", "yes, "1" vs. "false", "off", "no", "0" values, others expect just one of those.

This PR is about consolidating the code to one method that can be easily used with old and new settings (e.g. in #2807 discussion), including those where some other non-boolean values are also anticipated, e.g. return -1 for non-boolean (so it can be parsed further or considered non-false/non-true by situation), 0 for false, 1 for true.

It would also simplify configuration for people, so they would not have to keep consulting the man pages where a 0/1 is needed, and where a yes/no, etc.

NOTE: The C++ code already handles that generically, so I suspect nutconf might in fact emit configurations that could be unreadable to those lines in our daemons that expect only a certain value, e.g. "1" while a "true" is in their config and ignored. Never checked for that practically though.

Maybe we would also need to extend augeas definitions for fields that anticipated one specific pattern (e.g. 0/1) to be more omnivorous.

See also #31 about common boolean type definition.

@jimklimov jimklimov added augeas Configuration file parser (reader, writer) multi-tool for scripting, etc. C++ C-bool Issues and PRs about C/C++ methods, headers and data types dealing with boolean types C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks enhancement refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings labels Feb 23, 2025
@jimklimov jimklimov added this to the 2.8.4 milestone Feb 23, 2025
@jimklimov
Copy link
Member Author

Just a thought: some code mentions "enabled"/"disabled" values, should they also be thrown into the mix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
augeas Configuration file parser (reader, writer) multi-tool for scripting, etc. C++ C-bool Issues and PRs about C/C++ methods, headers and data types dealing with boolean types C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks enhancement refactor/fightwarn PR or issue proposal to improve code maintainability without functional changes, or to fix warnings
Projects
None yet
Development

No branches or pull requests

1 participant