Fix false-positive check for tty in flatpak #39

MattSturgeon · 2022-07-16T14:01:02Z

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 · 2022-07-22T02:07:45Z

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 · 2022-07-22T05:05:53Z

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.

Fix false-positive check for tty in flatpak

f5813fc

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

MattSturgeon force-pushed the flatpak-tty branch from 0a0b187 to f5813fc Compare July 16, 2022 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix false-positive check for tty in flatpak #39

Fix false-positive check for tty in flatpak #39

MattSturgeon commented Jul 16, 2022

0rax commented Jul 22, 2022

MattSturgeon commented Jul 22, 2022 •

edited

Loading

Fix false-positive check for tty in flatpak #39

Are you sure you want to change the base?

Fix false-positive check for tty in flatpak #39

Conversation

MattSturgeon commented Jul 16, 2022

0rax commented Jul 22, 2022

MattSturgeon commented Jul 22, 2022 • edited Loading

MattSturgeon commented Jul 22, 2022 •

edited

Loading