Skip to content

Use const and/or references for string arguments in RViz plugin where appropriate - #9

Merged
scastro-nasa merged 2 commits into
jazzy-develfrom
scastro/const-ref
Jun 12, 2026
Merged

scastro-nasa merged 2 commits into
jazzy-develfrom
scastro/const-ref

Conversation

@scastro-nasa

Copy link
Copy Markdown
Contributor

Requires NASA-JSC-Robotics/color_tools#8 to go in first

Tested on behavior demos in clr_ws and still works!

Comment on lines 215 to +225
@@ -222,7 +221,8 @@ void BehaviorTreeWidget::update_bt_text()
{
bt_print_text += "<br/>";
}
bt_print_text += process_text(bt_data);
process_text(bt_data);
bt_print_text += bt_data;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's better to eat the copy in the range-based for-loop than in all calls to process_text (and its internal multiple calls to replace_string ... I think.

@msavchen-nasa msavchen-nasa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good.
Side note Nathan said that there was a better way to do process_text but I havent had time to look into.
@scastro-nasa do you have any ideas?

@scastro-nasa

Copy link
Copy Markdown
Contributor Author

Looks good. Side note Nathan said that there was a better way to do process_text but I havent had time to look into. @scastro-nasa do you have any ideas?

I do! I think you can pre-define the list of all replacements and do them with std::string::replace -- let me try that real quick.

@scastro-nasa
scastro-nasa merged commit 01741de into jazzy-devel Jun 12, 2026
1 check passed
@scastro-nasa
scastro-nasa deleted the scastro/const-ref branch July 15, 2026 14: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