Skip to content

Commit

Permalink
set_color: don't set color to black before resetting attributes
Browse files Browse the repository at this point in the history
I was surprised to see:

> set_color normal | string escape
\e\[30m\e\(B\e\[m

I only expected to see a sgr0 here.

Cleanup a nearby `else { if (...) {` and comment with a bogus example.
  • Loading branch information
floam committed Jan 23, 2019
1 parent a17e6fa commit 09d0f77
Showing 1 changed file with 6 additions and 9 deletions.
15 changes: 6 additions & 9 deletions src/builtin_set_color.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,21 +216,18 @@ int builtin_set_color(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
}

if (bgcolor != NULL && bg.is_normal()) {
write_color(rgb_color_t::black(), false /* not is_fg */);
writembs_nofail(tparm((char *)exit_attribute_mode));
}

if (!fg.is_none()) {
if (fg.is_normal() || fg.is_reset()) {
write_color(rgb_color_t::black(), true /* is_fg */);
writembs_nofail(tparm((char *)exit_attribute_mode));
} else {
if (!write_color(fg, true /* is_fg */)) {
// We need to do *something* or the lack of any output messes up
// when the cartesian product here would make "foo" disappear:
// $ echo (set_color foo)bar
set_color(rgb_color_t::reset(), rgb_color_t::none());
}
} else if (!write_color(fg, true /* is_fg */)) {
// We need to do *something* or the lack of any output
// with a cartesian product here would make "foo" disappear
// on lame terminals:
// $ env TERM=vt100 fish -c 'echo (set_color red)"foo"'
set_color(rgb_color_t::reset(), rgb_color_t::none());

This comment has been minimized.

Copy link
@floam

floam Jan 23, 2019

Author Member

This old fix of mine is lame, we probably don't want to actually cause the attributes to be reset. Also, this would still be broken for --background because we only did it for the fg case. I should probably output a real no-op like a NUL or DEL.

This comment has been minimized.

Copy link
@faho

faho Jan 23, 2019

Member

Or... you know... a newline? See #5443.

}
}

Expand Down

0 comments on commit 09d0f77

Please sign in to comment.