search

tinted-theming / tinted-shell

Base16 and Base24 for Shells
https://github.com/tinted-theming/home
MIT License
65 stars 217 forks source link

template: print escape codes directly to the tty #52

Closed jdrouhard closed 8 months ago

jdrouhard commented 10 months ago

Instead of relying on the script's stdout to be connected to the tty, just bypass stdout and pipe the termcodes directly to the tty.

If the scripts are run in a context where stdout has been redirected (to a file, or to /dev/null, etc), then these escape codes are lost. If the terminal is connected to a TTY, though, there's no reason these escape codes can't be written there anyway.

joshbode commented 8 months ago

I like this idea - I do something similar with my shell functions that need to return escapes to tmux and friends.

One thing to be careful of - if stdin is not open, then tty will return an error (and the text not a tty), e.g.

$ tty < /dev/null
not a tty

$ echo $?
1

This will be a problem if the output is redirecting to that filename - you'll end up with files with the name not a tty being created in random locations :)

Also, some shells (zsh at least) already set a variable TTY with the same value as tty returns.

How about adding a check like this?

if [ -z "$TTY" ] && ! TTY=$(tty); then
  TTY=/dev/null
fi

Additionally, it would be good practice to quote the $TTY variable, e.g.

... > "$TTY"

even though with the check above there should never be any spaces in the value

joshbode commented 8 months ago

Oops, sorry! I completely missed the check after invoking tty:

if [ $? -ne 0 ]; then

so that case is covered :)

shellcheck has bullied me into not using $? for too long, I guess! (https://www.shellcheck.net/wiki/SC2181)

You could also write it as:

if [ -z "$TTY" ] && ! TTY=$(tty); then
  put_template() { true; }
  put_template_var() { true; }
  put_template_custom() { true; }
...
joshbode commented 8 months ago

Also, do you still need to commit the template?

jdrouhard commented 8 months ago

Also, do you still need to commit the template?

Yes, somehow that file didn't get added to my commit initially. Thanks for noticing!

I also took your other suggestion to not rely on $? for the previous command's return code and I think it cleaned it up quite a bit.

JamyGolden commented 8 months ago

Hey @jdrouhard, I wasn't tagged as a reviewer so missed this. Could you rebase and build themes again?

jdrouhard commented 8 months ago

Hey @jdrouhard, I wasn't tagged as a reviewer so missed this. Could you rebase and build themes again?

Done.

JamyGolden commented 8 months ago

Thanks for the changes :pray: