-
Notifications
You must be signed in to change notification settings - Fork 4.9k
derive context settings from configuration file #12432
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
Changes from all commits
1c6cd31
55ee86e
e2741af
5d6d09e
ca82dce
f2a9e91
6cec1da
2361208
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| // License: Apache 2.0. See LICENSE file in root directory. | ||
| // Copyright(c) 2017 Intel Corporation. All Rights Reserved. | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <nlohmann/json.hpp> | ||
|
|
||
| #include <map> | ||
| #include <string> | ||
| #include <sstream> | ||
|
|
@@ -40,7 +41,7 @@ namespace rs2 | |
| { | ||
| public: | ||
| config_file(); | ||
| config_file(std::string filename); | ||
| config_file( std::string const & filename ); | ||
|
|
||
| void set_default(const char* key, const char* calculate); | ||
|
|
||
|
|
@@ -91,8 +92,8 @@ namespace rs2 | |
|
|
||
| void save(); | ||
|
|
||
| std::map<std::string, std::string> _values; | ||
| std::map<std::string, std::string> _defaults; | ||
| std::string _filename; | ||
| nlohmann::json _j; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the underlying json object. I can name it _json, but I don't know if that adds much... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name should not change if we decide to switch from JSON to XML or other. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. _data? _map? _key_value? Anything you pick is meaningless. :) |
||
| }; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Newline needed? |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why check for string? Don't we support other values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because, at least in rs-config, and in this PR, I didn't want to change any existing functionality: tools will keep behaving the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main goal was to make rs-config compatible with raw json -- before, it couldn't even load -- and able to keep non-string content intact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in the next PR this is about to change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I'm not touching it again without a good reason.