isatty stdin not working #1443

Closed
bpinto opened this Issue Apr 30, 2014 · 4 comments

Comments

Projects
None yet
4 participants
@bpinto
Contributor

bpinto commented Apr 30, 2014

➜ isatty --help

Description

isatty tests if a file or file descriptor is a tty. The argument may be
in the form of a file path, device, or file descriptor number. Without
an argument, standard input is implied.

With stdin as argument:

➜ .oh-my-fish git:(Paulloz-plugin/sprunge) ✗ echo 'foo' | if isatty stdin
                                                     echo 'if'
                                             else
                                                     echo 'else'
                                             end
if

Without an argument:

➜ .oh-my-fish git:(Paulloz-plugin/sprunge) ✗ echo 'foo' | if isatty
                                                     echo 'if'
                                             else
                                                     echo 'else'
                                             end
else

Shouldn't both have the same behaviour? Installed fish from Git (today):

  • 1ffa4d9 - (HEAD, origin/master, origin/HEAD, master) openssl: add instructions for using custom certs (6 days ago)

bpinto added a commit to oh-my-fish/oh-my-fish-legacy that referenced this issue Apr 30, 2014

@bpinto bpinto referenced this issue in oh-my-fish/oh-my-fish-legacy Apr 30, 2014

Closed

add sprunge plugin #131

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish May 1, 2014

Member

We're being bitten by the change in #1061. In isatty:

if test -e /dev/"$argv"
    command test -c /dev/"$argv"; and tty 0>/dev/"$argv" >/dev/null
...

The exit status of the command gets swallowed by the if statement, so it doesn't get propagated back to the caller.

Member

ridiculousfish commented May 1, 2014

We're being bitten by the change in #1061. In isatty:

if test -e /dev/"$argv"
    command test -c /dev/"$argv"; and tty 0>/dev/"$argv" >/dev/null
...

The exit status of the command gets swallowed by the if statement, so it doesn't get propagated back to the caller.

@ridiculousfish

This comment has been minimized.

Show comment
Hide comment
@ridiculousfish

ridiculousfish May 1, 2014

Member

I think the fix in #1061 is wrong. I made the change to have if statements always set the exit status to 0. But this is not what zsh does:

zsh> if false ; then ; else echo inside: $? ; fi ; echo $?

This outputs:

inside: 1
0

so the if condition is visible within the if statement, but not outside of it.

I can add an extra 'false' command like so:

zsh> if false ; then ; else echo inside: $? ; false ; fi ; echo $?

Now the exit status is 1 upon exit, because of the inner false.

Member

ridiculousfish commented May 1, 2014

I think the fix in #1061 is wrong. I made the change to have if statements always set the exit status to 0. But this is not what zsh does:

zsh> if false ; then ; else echo inside: $? ; fi ; echo $?

This outputs:

inside: 1
0

so the if condition is visible within the if statement, but not outside of it.

I can add an extra 'false' command like so:

zsh> if false ; then ; else echo inside: $? ; false ; fi ; echo $?

Now the exit status is 1 upon exit, because of the inner false.

@geoff-codes

This comment has been minimized.

Show comment
Hide comment
@geoff-codes

geoff-codes Jan 21, 2015

Contributor

I think the fix in #1061 is wrong.

Yes, it is. Per POSIX:

The if Conditional Construct

The if command shall execute a compound-list and use its exit status to determine whether to execute another compound-list.

The format for the if construct is as follows:

if compound-list
then
    compound-list
[elif compound-list
then
    compound-list] ...
[else
    compound-list]
fi

The if compound-list shall be executed; if its exit status is zero, the then compound-list shall be executed and the command shall complete. Otherwise, each elif compound-list shall be executed, in turn, and if its exit status is zero, the then compound-list shall be executed and the command shall complete. Otherwise, the else compound-list shall be executed.

Exit Status

The exit status of the if command shall be the exit status of the then or else compound-list that was executed, or zero, if none was executed.

So its not that it eats the return status. It returns 0 if the the if/else if conditional(s) are non-zero, otherwise, it takes the return value of the commands executed.

Contributor

geoff-codes commented Jan 21, 2015

I think the fix in #1061 is wrong.

Yes, it is. Per POSIX:

The if Conditional Construct

The if command shall execute a compound-list and use its exit status to determine whether to execute another compound-list.

The format for the if construct is as follows:

if compound-list
then
    compound-list
[elif compound-list
then
    compound-list] ...
[else
    compound-list]
fi

The if compound-list shall be executed; if its exit status is zero, the then compound-list shall be executed and the command shall complete. Otherwise, each elif compound-list shall be executed, in turn, and if its exit status is zero, the then compound-list shall be executed and the command shall complete. Otherwise, the else compound-list shall be executed.

Exit Status

The exit status of the if command shall be the exit status of the then or else compound-list that was executed, or zero, if none was executed.

So its not that it eats the return status. It returns 0 if the the if/else if conditional(s) are non-zero, otherwise, it takes the return value of the commands executed.

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey Jan 21, 2015

Member

I think we should try and fix this for 2.2.

Member

zanchey commented Jan 21, 2015

I think we should try and fix this for 2.2.

@zanchey zanchey modified the milestones: next-minor, next-major Jan 21, 2015

geoff-codes added a commit to geoff-codes/fish-shell that referenced this issue Jan 21, 2015

geoff-codes added a commit to geoff-codes/fish-shell that referenced this issue Jan 21, 2015

geoff-codes added a commit to geoff-codes/fish-shell that referenced this issue Jan 21, 2015

geoff-codes added a commit to geoff-codes/fish-shell that referenced this issue Feb 23, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment