feat(echo --clear): add --clear option to echo #819
Conversation
Signed-off-by: Guillaumebeuzeboc <[email protected]>
| to_print = '---Got new message, message info:---\n' | ||
| to_print = f'{to_print}{info}\n' | ||
| to_print = f'{to_print}---Message data:---\n' | ||
| print(f'{self.prefix}{to_print}{submsg}', end='\n---\n') |
There was a problem hiding this comment.
having escape sequence was what we do in ROS 1 rostopic echo, so this would be probably good.
instead, how about taking advantage of os.system() to clear the screen? so that we can just rely on system shell that gives us more flexibility for the future?
import os
import system
if platform.system() == 'Windows':
os.system('cls')
else:
os.system('clear')
There was a problem hiding this comment.
I agree that the escape sequence is potentially problematic, particularly on Windows.
That said, I'm really not to enamored of doing os.system. That almost always leads to trouble down the line, as it is difficult to deal with errors and easy to trick it into doing something nefarious.
I think it is worthwhile here to step back and maybe look at some of the python command-line libraries and see what they do in this situation. If we can find a solution to work on all shells and in Windows (both cmd and powershell), and does not require os.system, that would be the ideal.
I had the same failure with the HEAD of rolling for this repo. I had to rebuild all rolling from source for them to pass. |
Yes, that's correct, though as of yesterday, they are the same thing. That said, these tests have indeed been flaky here for a while, and we haven't had time to track it down. So I wouldn't worry about those two in particular right now. |
clalancette
added
more-information-needed
|
Does that PR require further changes? |
Yes, we need to come to a conclusion on this discussion. |
Adds the
--clear/-coption to echo.Solves this issue: #725
This was previously available in ROS.
This clears the screen before a new message is displayed.
ESC[2Jcorresponds toerase entire screenESC[Hcorresponds tomoves cursor to home position (0, 0)I had to build rolling from source to successfully run the tests