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

-C uses last pipe colour from previous invocation #22

Closed
parkercoates opened this issue Sep 16, 2015 · 3 comments
Closed

-C uses last pipe colour from previous invocation #22

parkercoates opened this issue Sep 16, 2015 · 3 comments
Assignees
Labels

Comments

@parkercoates
Copy link
Contributor

If I run pipes.sh; pipes.sh -C, instead of drawing without colour, the second invocation always draws with the last colour drawn from the first invocation. Obviously some terminal escape state is lying around. This seems related to, but distinct from, #5.

I'm no expert on tput and friends, but I see two potential ways of fixing this, which both seem to work.

  1. Insert a call to tput reset just before entering the main animation loop. This ensures that drawing always starts from a clean state.

    stty -echo
    tput reset # <<<<<<<<<<<<<<<<<<<<< Added line
    tput smcup || FORCE_RESET=1
    tput civis
    tput clear
    # any key press exits the loop and this script
    while REPLY=; read -t 0.0$((1000/f)) -n 1 2>/dev/null; [[ -z $REPLY ]] ; do
    
  2. Insert a call to tput reset into cleanup. This ensures that the script always cleans up after itself.

    cleanup() {
        # clear up standard input
        read -t 0.001 && cat </dev/stdin>/dev/null
    
        # terminal has no smcup and rmcup capabilities
        ((FORCE_RESET)) && reset && exit 0
    
        tput reset # <<<<<<<<<<<<<<<<<<<<< Added line
        tput rmcup
        tput cnorm
        stty echo
        ((NOCOLOR)) && echo -ne '\e[0m'
        exit 0
    }
    

Personally I think the second option seems more responsible, but it could be that there's a third option that makes more sense. Again, I'm no expert on this stuff.

@livibetter
Copy link
Contributor

Using st-0.7 and xterm-297 on Linux, this issue doesn't happen, @parkercoates, what's your OS and terminal emulator?

As you have mentioned #5, 3816faf was made to fix this issue, this should be the same to be fixed in #5, apparently it didn't fix for all. What exactly did you do to reproduce the bug? Run the command and immediately press Q, I even tried

./pipes.sh -p100 -r0 -R; ./pipes.sh -C

to fill up the screen, but still can't reproduce.

If it's confirmed by others, then we could take tput reset approach and revert that commit.

@parkercoates
Copy link
Contributor Author

@livibetter, you're right that this issue is dependent on the environment. I can reproduce the issue with konsole, but xterm and gnome-terminal don't seem to be affected. I have no idea whether this is actually a bug in Konsole or an unspecified difference in behaviour. As I said, I'm no expert on terminal control codes.

If you wanted to give me a minimal test case to try, I'd be happy to take the issue up with the Konsole developers.

@livibetter
Copy link
Contributor

@parkercoates I can't give you a minimal test case since I don't have KDE/konsole to create one with. However, since it's konsole, it should have good amount of users, I'd vote for the tput reset workaround providing it won't break other emulators else, which I don't believe it would. You should submit another PR and specially refer to this issue in comment.

I also wonder what if #34 is also the same, all depending on the environment.

This repo needs an issue report template/form for those information to be filled in when reporting.

@livibetter livibetter self-assigned this Feb 23, 2018
livibetter added a commit to lbforks/pipes.sh that referenced this issue Feb 23, 2018
The second proposed method, by @parkercoates in pipeseroni#22, can be implemented
safely for terminal emulators which are not affected by the issue.
livibetter added a commit to lbforks/pipes.sh that referenced this issue Feb 23, 2018
The second proposed method, by @parkercoates in pipeseroni#22, can be implemented
safely for terminal emulators which are not affected by the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants