-
Notifications
You must be signed in to change notification settings - Fork 112
Adds {severity_with_color} as a log format option. #526
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
base: rolling
Are you sure you want to change the base?
Conversation
Signed-off-by: Alex MacLean <[email protected]>
4cad812 to
b7a4469
Compare
Signed-off-by: Alex MacLean <[email protected]>
Co-authored-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Alex MacLean <[email protected]>
Signed-off-by: Alex MacLean <[email protected]>
0b8c24c to
8674a60
Compare
|
@ahcorde waffle team is assigning this to you. Commenting so I see when it gets merged so I can share it with the community. |
ahcorde
left a comment
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.
Do you mind to add some documentation on ros2_documentation about how to use this new feature ?
|
Pulls: #526 |
Happy to. I'll update the documentation on Monday, and create a pull request to |
|
Added documentation: ros2/ros2_documentation#6045 |
|
@ahcorde - I'm trying to decipher the build failures that you listed. Are those build infrastructure errors? I don't see compile or test failures in the console output. I only see IO Exceptions for Jenkins and Hudson. |
|
Pulls: #526 |
fujitatomoya
left a comment
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.
@alexmaclean6 are you still working on this? if you are, can you address the CI failures?
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
|
Pulls: #526 |
| return logging_output->buffer; | ||
| } | ||
|
|
||
| // Forward declare expand_serverity_with_color to associate it with tokens. |
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 think this is fine to do that, but not consistent with other handlers that are implemented in here with declaration so that it can get the function pointers to the token table.
| // If the severity is 4 characters long, add another space to line it up with the | ||
| // 5 character severities. | ||
| if (strlen(severity_string) == 4) { | ||
| if (rcutils_char_array_strcat(logging_output, " ") != RCUTILS_RET_OK) { | ||
| RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string().str); | ||
| rcutils_reset_error(); | ||
| RCUTILS_SAFE_FWRITE_TO_STDERR("\n"); | ||
| return NULL; | ||
| } | ||
| } |
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 we have this padding space only for this handler? i do not think this is consistent behavior for logging format?
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.
@ahcorde what do you think?
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.
@fujitatomoya you are more familiar with the logging system. @alexmaclean6 do you mind to address @fujitatomoya comment?
| [\033[0mINFO \033[0m] [name]: Info message (func() at file:42) | ||
| [\033[33mWARN \033[0m] [name]: Warn message (func() at file:42) |
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.
These need to be without padding space with other console format?
| [\033[0mINFO \033[0m] [name]: Info message (func() at file:42) | |
| [\033[33mWARN \033[0m] [name]: Warn message (func() at file:42) | |
| [\033[0mINFO\033[0m] [name]: Info message (func() at file:42) | |
| [\033[33mWARN\033[0m] [name]: Warn message (func() at file:42) |
Description
This change adds a new log formatting token option:
{severity_with_color}. When used in the log format string, the severity level will be colorized in the log output, but the rest of the line will not be affected. IfRCUTILS_COLORIZED_OUTPUT=1, causing the entire line to be colorized,{severity_with_color}behaves the same as{severity}.Example usage:
Is this user-facing behavior change?
Yes. It allows users to use
{severity_with_color}in their log format.Did you use Generative AI?
No
Additional Information