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

Add support for specifying both foreground and background colors. #49

Merged
merged 11 commits into from
Nov 15, 2019
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
*.swp
*~
mjarkk marked this conversation as resolved.
Show resolved Hide resolved
78 changes: 49 additions & 29 deletions escape.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,39 +191,59 @@ func (ei *escapeInterpreter) output256() error {
return ei.outputNormal()
}

fgbg, err := strconv.Atoi(ei.csiParam[0])
if err != nil {
return errCSIParseError
}
color, err := strconv.Atoi(ei.csiParam[2])
if err != nil {
return errCSIParseError
}

switch fgbg {
case 38:
ei.curFgColor = Attribute(color + 1)
for _, param := range ei.splitFgBg() {
fgbg, err := strconv.Atoi(param[0])
if err != nil {
return errCSIParseError
}
color, err := strconv.Atoi(param[2])
if err != nil {
return errCSIParseError
}

for _, param := range ei.csiParam[3:] {
p, err := strconv.Atoi(param)
if err != nil {
return errCSIParseError
switch fgbg {
case 38:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we change this magic number stuff while we are at it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a change that I think is what you want.

ei.curFgColor = Attribute(color + 1)

for _, s := range param[3:] {
p, err := strconv.Atoi(s)
if err != nil {
return errCSIParseError
}

switch {
case p == 1:
ei.curFgColor |= AttrBold
case p == 4:
ei.curFgColor |= AttrUnderline
case p == 7:
ei.curFgColor |= AttrReverse

}
}
case 48:
ei.curBgColor = Attribute(color + 1)
default:
return errCSIParseError
}
}
return nil
}

switch {
case p == 1:
ei.curFgColor |= AttrBold
case p == 4:
ei.curFgColor |= AttrUnderline
case p == 7:
ei.curFgColor |= AttrReverse
}
func (ei *escapeInterpreter) splitFgBg() [][]string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this function need to be a method of ei, cant this just be a function we can call like

func splitFgBg() [][]string { ... return out}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spltFfBg is very specific to the escapeInterpreter, so to me it makes sense to keep it as is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that, but it does not rely on ei, it just relies on data. It is not driven by the struct

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal to me. I'll change it. Just a sec.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

var out [][]string
var current []string
for _, p := range ei.csiParam {
if len(current) > 0 && (p == "48" || p == "38") {
out = append(out, current)
current = []string{}
}
case 48:
ei.curBgColor = Attribute(color + 1)
default:
return errCSIParseError
current = append(current, p)
}

return nil
if len(current) > 0 {
out = append(out, current)
}

return out
}