starship / starship

☄🌌️ The minimal, blazing-fast, and infinitely customizable prompt for any shell!
https://starship.rs
ISC License
45.53k stars 1.98k forks source link

Respecting TERM=dumb #1588

Open Dietr1ch opened 4 years ago

Dietr1ch commented 4 years ago

Feature Request

Is your feature request related to a problem?

Emacs' tramp-mode opens remote files nicely through ssh, but for that to work properly you need to have a simple shell prompt that tramp understands, otherwise it ends up hanging on unexpected input.

To avoid chocking on fancy prompts, tramp uses the TERM=dumb environment variable to tell the shell that it can barely understand a prompt like $.

This makes properly customizing the prompt require checks like this one. If you look at the commit, you'll see that they added the check and it seems to work, but they only fixed bash and since my default shell is fish, it'd take another pull-request to get right, and yet another if zsh was also broken.

Describe the solution you'd like

I'd like starship to be aware of dumb terminals and just do the right thing in case the shell configuration is not set up properly. I'd rather have starship init $SHELL configure the prompt in the right way for me.

Describe alternatives you've considered

Properly configuring the shell and not calling starship init on dumb terminals. It's possible and maybe a better solution, but if we can detect the config got broken, then providing a fallback instead of triggering a working-as-intended breakage seems friendlier.

chipbuster commented 4 years ago

Looking around, it appears to me that TERM=dumb has specific meanings outside of emacs as well. In particular, dumb has an entry in the terminfo database which specifies a limited set of capabilities. Wikipedia has a short summmary as well.

80 cols (no autowrap), and almost no ANSI escape sequences...I don't think we have a good way to configure this output in our current prompt printer, because each module generates its own output as a string (complete with ANSI color escape sequences) before passing it to the main starship code to be laid out/printed.

The Right Way™ to do this would be to wait for the starship refactor to come out, which uses an in-memory representation of the module instead of strings, then create a backend specifically for dumb terminals. But that likely won't happen until next year at the earliest.

As a fast-and-dirty alternative, we could have the rendering phase of the prompt go through and strip out all ANSI escape sequences, similar to how we currently hack in zero-length escape sequences for bash/zsh.

Dietr1ch commented 4 years ago

Is that refactor really needed? I thought we could just modify the initialization scripts. Something around,

https://github.com/Dietr1ch/starship/compare/master...Dietr1ch:dumb-term (github's differ fails to show the code moves :cry:)

i'm just setting the prompt to $ instead of setting up starship if TERM=dumb is detected. It won't setup each shell to fully respect the terminfo setup, but it'll stop from making things worse by setting up a nice prompt.

chipbuster commented 4 years ago

So I guess the disagreement here is in this statement:

I'd like starship to be aware of dumb terminals and just do the right thing in case the shell configuration is not set up properly.

What is "the right thing" when starship sees TERM=dumb? Is the correct behaviour just to disable itself, or to display as much information as is practical for those terminal capabilities?

Several search results suggest to me that some people actually interact with terminals in dumb mode (maybe I'm misreading them? see here and here), and I think it would be confusing at best to run starship init and get no prompt.

davidkna commented 4 years ago

Doesn't explain get the (length of) modules without styling with module.get_segments().join(""); or does that do something else?

chipbuster commented 4 years ago

@davidkna I think so, but the module output itself still seems to be colored (at least on my copy of master).

Dietr1ch commented 4 years ago

What is "the right thing" when starship sees TERM=dumb? Is the correct behaviour just to disable itself, or to display as much information as is practical for those terminal capabilities?

I see your point, thanks for clarifying things :)

I think that both goals are the right thing to do in different timeframes. If we only had an hour to "fix" this, we should just make starship disable itself, as the current state definitely breaks dumb terminals. However, your goal of making it as useful as possible is the right end-goal.

chipbuster commented 4 years ago

@Dietr1ch The current starship implementation likely won't work in dumb terminals, so I can see the need for a hotfix. However, scattering it all over the init scripts seems a little haphazard to me.

What if we throw in a patch into starship itself that just prints a something like starship disabled due to TERM=dumb > as the prompt? All ASCII, no escape sequences, and anyone who triggers it by accident will at least have a clue as to what's going on.

Then we can leave this issue open as a tracker for how to display starship when we don't have smart terminal capabilities and work towards eventually getting that right.

Dietr1ch commented 4 years ago

Sounds good!

Yeah, with the long term plan it seems bad to scatter that hack around all the init scripts.

chipbuster commented 4 years ago

Doesn't explain get the (length of) modules without styling with module.get_segments().join(""); or does that do something else?

@davidkna Sorry, I just understood what you meant :facepalm: Yes, explain can get the module text without ANSI styling (I had forgotten this was possible after the format strings update). I still don't know if that's enough for dumb terminals, since the prompt will still have emoji and powerline symbols. If we can get by without stripping those out, I think that will be good enough to do the whole thing at once.

andytom commented 3 years ago

This was fixed in #1594.

Yep, I didn't read all the comments that was just a hot fix until we can fix this properly.

Dietr1ch commented 3 years ago

The Right Way™ to do this would be to wait for the starship refactor to come out, which uses an in-memory representation of the module instead of strings, then create a backend specifically for dumb terminals. But that likely won't happen until next year at the earliest.

As a fast-and-dirty alternative, we could have the rendering phase of the prompt go through and strip out all ANSI escape sequences, similar to how we currently hack in zero-length escape sequences for bash/zsh.

We are currently stuck here, between breaking TRAMP or (soft-breaking) eshell due to not being able to correctly fallback to a dumb mode while retaining most of the information we want the prompt to show.

I'd say that losing the customization under eshell is better than making tramp-mode hang forever. Both issues happen under Emacs, but I'd hate to have emacs-specific checks in starship, so the current state seems closer to the best that we can get for now.

Until segments become an intermediate representation that gets rendered to a modern or dumb terminal there's no nice solution. In the meantime changing "Starship disabled due to TERM=dumb > " for something more useful seems to be the way to go, we only need to warn once about the diminish capabilities under dumb terminals.

codygman commented 3 years ago

In shell-mode the prompt of:

[ERROR] - (starship::print): Under a 'dumb' terminal (TERM=dumb).
Starship disabled due to TERM=dumb > 

Is annoying as an emacs user. I guess I can probably remove starship without too much trouble though so maybe my annoyance shouldn't count for much.

I'm moving away from using a terminal and replacing everything with emacs anyway so :man_shrugging:

mattray commented 3 years ago

@codygman I added this in my .bashrc to get around it

if [ "dumb" == "$TERM" ] ; then
  export TERM=xterm-256color
fi