erkin / ponysay

Pony rewrite of cowsay.
GNU General Public License v3.0
1.21k stars 81 forks source link

ponysay leaves terminal bold after running help command #241

Open CrazyPython opened 8 years ago

CrazyPython commented 8 years ago

Platform: OSX

Ponysay leaves the terminal with bold. I think it's missing a control character. (The text after running ponysay is all in bold)

JotaRandom commented 8 years ago

I dont have an OSX to test but... I know that in TTY in linux the bold text is also leaved with a different colour.

Do you know what is the "control code" used in OSX to return the bold color to the default?

CrazyPython commented 8 years ago

@jristz same control code as other POSIX terminals - I've dealt with this before, a few google searches should come up with the reset escape

JotaRandom commented 8 years ago

I'm testing here in a linux tty and in a unix terminal but no one left the the terminal bold when I use help.

In theory the standard control code showl work but I dont get the problem.

Do you mentioned that his happend on OSX, right? so you you can share some screenshoot or the control code used in the OSX terminal to clean the blodness (just to test that here you know, testing to reach out of the issue)

hashhar commented 7 years ago

Does OSX have a tput command? If yes you can use something like tput sgr0 for resetting, tput bold for bold.

cb="$(tput bold)"
cc="$(tput sgr0)"

# Then use them
echo "${cb}BOLD${cc} and normal."
ngyikp commented 7 years ago

Yeah, tput sgr0 works on Mac 👍

screen shot 2017-04-08 at 6 14 30 pm

At https://github.com/erkin/ponysay/blob/f65be435acffda57b1e7681dc3152d1afdc082ac/src/argparser.py#L268 , we use code 21 to reset the bold style.

Looking at the node.js world, they use 22 instead, saying "21 isn't widely supported and 22 does the same thing" https://github.com/chalk/ansi-styles/blob/b559ed61bd09d21cda8fced733c39e4f83fb06bc/index.js#L23

I confirm that changing it to 22 fixes the bug: https://github.com/erkin/ponysay/compare/master...ngyikp:fix-bold

screen shot 2017-04-08 at 6 44 46 pm
JotaRandom commented 7 years ago

I tested it and I dont find problem...

so I docided make the pull request as #257