-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
add slice and shorten_json_string #11598
Conversation
// find an opening | ||
slice::const_iterator it = outside.begin() + 1; // opening {, separating comma, etc. | ||
bool in_quote = false; | ||
while( it < outside.end() && ( in_quote || *it != '[' && *it != '{' ) ) |
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.
It is better to move *it != '[' && *it != '{'
to a separate function is_opening
, the meaning will be clearer.
For the very least surround with parenthesis, don't count on operator &&
precedence over ||
.
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.
I don't like parens that aren't needed generally... I'll try to add is_opening
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.
Fixed
const_iterator _begin, _end; | ||
|
||
public: | ||
slice( const_iterator begin, const_iterator end ) |
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.
It is better to leave a blank line between functions.
Fix if you'll have some other changes to this file.
return ellipsis( slice(), str ); // impossible: "{ ... }" is the minimum | ||
|
||
// Try to find an inside block that can be taken out | ||
ellipsis final; |
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.
final
can serve as a C++ keyword in other context, better to use other name. result
?
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.
fixed
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.
LGTM
This is a utility function I used in one PoC that thought would be useful to have in rsutils, especially now that we're making a bigger use of json.
It also adds what I consider a very useful utility
slice
, similar to astring_view
. For example, I wanted to refer to the json string insideflexible_msg
but it's not null-teminated. The easy way is by using a slice:LOG_DEBUG( "publishing metadata: " << shorten_json_string( slice( md.custom_data< char const >(), md._data.size() )));
Anyway I didn't want to lose this so decided to PR.