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

Fix false-positive check for tty in flatpak #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MattSturgeon
Copy link

When there is absolutely no tty set at all (e.g. in a shell spawned by
a flatpak), tty prints not a tty, which matches the current check tty | grep "tty".

In normal shells it prints something like /dev/pts/0, in actual ttys it prints something like /dev/tty1.

Luckily tty also returns 1, so we can do a two-stage check instead:

# Capture tty's output in a local variable
# set doesn't modify the return status of tty
set --local out (tty)

# If tty returned non-zero, search its output
and string match --quiet --entire "tty" $out
  • A more robust check might use a regex to check:
string match --quiet --regex --entire '/ttyS?\d+$' $out

I figured it's better to use the faster substring matching rather than regex matching, especially if this is run every time a prompt is generated, but if not - or if you'd rather be more robust - let me know.

This fixes #38

When there is absolutely no tty set at all (e.g. in a shell spawned by
a flatpak), tty prints 'not a tty', which matches `tty | grep "tty"`.

Luckily `tty` also returns `1`, so we can do a two-stage check instead.

This fixes 0rax#38
@0rax
Copy link
Owner

0rax commented Jul 22, 2022

Hello @MattSturgeon, thanks for your contribution. I can't really test everything right now and will take a look at it during the weekend.

At a quick glance and by reading your comment, we might want to go with a regexp match here if it is more robust as this part of the code is only called during the initialization of fishline (one per shell process).

I am also wondering if there might be a way to remove the special case for macOS ([ (uname) != "Darwin" ]) using such a check. A colleague reported that a Terminal.app session on macOS returns /dev/ttys000 which is kind of a pain if we are using any regex matching mechanism.

The safe way might be to keep this special case for Darwin and just use your current regex suggestion as a sanity check.

@MattSturgeon
Copy link
Author

MattSturgeon commented Jul 22, 2022

This stackexchange answer seems to indicate that on OS X the terminals will always be running in its own tty (e.g. /dev/ttys000 or /dev/ttys001). Apparently there's no pty concept on Darwin, only console and ttys.

On linux (at least openSUSE) ls /dev/tty* reveals /dev/tty0 (no s, no leading zeros) and /dev/ttyS0 (uppercase s, no leading zeros).

We could match against not having a lowercase s, but IMO this could introduce edge cases for any (BSD?) systems using that format. Not sure. It's also possible that such systems also don't have ptys, and so would use TTYS for everything like on OS X...

Perhaps we could somehow test if the system supports ptys before checking if we are on a tty? Maybe just ls /dev/pty* or test -e 🤔

Or perhaps there's some other way we can check if powerline symbols are available? I don't know if any reliable way to get the terminal's font... Some terminals set an environment variable to their name (IIRC TERMINAL)... 🤔

EDIT: this discussion clarifies a few points. Seems macos uses a newer ptm system to assign ttys000 psudo terminals.

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.

TTY theme is used in flatpak terminals
2 participants