Skip to content
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 RTW “header” to terminal with all data #134

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

atticusrussell
Copy link

addresses issue #115 Add RTW "header" to terminal with all data
additionally splits apart prompt construction in a way that is simpler to understand

new behavior:
image

@atticusrussell atticusrussell marked this pull request as ready for review May 25, 2023 20:42
@destogl
Copy link
Member

destogl commented May 30, 2023

Hi @atticusrussell thanks for your contribution! This definitely goes into the direction I was thinking, but it is not exactly that. I was hoping to have at the top of the terminal, this filed will all information and not in each line. Nevertheless, let's see what other think on this solution.

A few nit picks:

  • Keep also other setups also in the file – we will probably let users choose from those when setting up the RTW.
  • we need to split better the content in the first line, like having it more centered and to the right. Also a bit indenation would help for the visibility.
  • can we keep the output stil if there there is no branch or workspace? (just adding for example <no git>, or [no workspace]

@StoglRobotics/stoglrobotics-non-client-repositories what do you think about this?

@gwalck
Copy link
Contributor

gwalck commented Jun 1, 2023

First thanks for this PR.

I am discussing the concept, not the implementation.

so basically it goes from 3 long lines with direct result above prompt

myname@mymachine:[myworkspace]<mybranch>lastfolderofPWD$ ls
CMakeLists.txt  config  launch  package.xml  src  test  toto  urdf
myname@mymachine:[myworkspace]<mybranch>lastfolderofPWD$

to 8 lines with the result below the previous promp but 4 lines above the current prompt and the last command also a bit lost in the 3 line.

myname@mymachine [myworkspace] <mybranch>
full_length_pwd_which_can_be_very_long_and_use_the_whole_space
18:54:46 $ ls
CMakeLists.txt  config  launch  package.xml  src  test  toto  urdf

myname@mymachine [myworkspace] <mybranch>
full_length_pwd_which_can_be_very_long_and_use_the_whole_space
18:54:49 $

I can give my opinion about this concept: I think a header in 2 lines per line won't work for me. I prefer a short line indeed, but was quite ok with the current state (indeed half a terminal).

I need to see if the initial idea would work better.
However, with a header at the top of the terminal, does that mean then that when scrolling the current folder would stay on top of the terminal as well ?
how do we know in which folder were previous commands executed when scrolling if there is no info about the last folder at least in the prompt line ?
I would prefer the static info at the top of the terminal and current last folder (without time) at every line before the prompt.

myname@mymachine [myworkspace] <mybranch>
lastfolderofPWD$ ls
CMakeLists.txt  config  launch  package.xml  src  test  toto  urdf
lastfolderofPWD$ cd src
src$ ls
some files
src$ 

@destogl
Copy link
Member

destogl commented Jun 4, 2023

@gwalck what I understood, there is a possibility to create static content at the top of the terminal that should not break any scrolling. Maybe I have understood something wrong, but that would be the idea to realize.

@destogl
Copy link
Member

destogl commented Jun 4, 2023

@atticusrussell do you have some time to check this?

@atticusrussell
Copy link
Author

@destogl yes I have time to work on it. This is my first PR in this repo so not entirely sure what my next step should be:

Should I modify the code to match the example at the end of @gwalck 's feedback to create a static header?

Also personally, I'm not a huge fan of keeping [no-workspace] or as you mentioned, and I prefer showing the whole path to the current dir, as opposed to only the last folder.

Does it make sense that I create boolean variables that enable/disable those features, and toggle between full path and last folder? I could make the default states of them match what you and @gwalck requested.

@destogl
Copy link
Member

destogl commented Jun 14, 2023

@atticusrussell yes you can implement this what @gwalck proposed. But we should add in the second line in header the full path. It is often annoying to me to have a long path ahead of command. And yes, we can add flag to enable/disable this - this is the easiest part :)

@atticusrussell
Copy link
Author

@destogl @gwalck I will work on implementing this. I think I'll need to use tmux to achieve the static header, unless anyone knows another way.

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