Support zero parameteter 'CSI n m' in parseOne #98
Conversation
This PR uses the jsoncolor package to colorize JSON bodies. It changes the imports to use my forked version of github.com/jroimartin/gocui which includes a small patch to support zero-parameter CSI escape sequences. I have submitted a PR upstream: jroimartin/gocui#98 Once that PR is merged the import can be switched back to github.com/jroimartin/gocui if desired.
|
I'll add that the reason I ran into this was because github.com/fatih/color outputs a zero-parameter CSI escape sequence |
escape.go
Outdated
| @@ -99,6 +99,11 @@ func (ei *escapeInterpreter) parseOne(ch rune) (isEscape bool, err error) { | |||
| } | |||
| return false, errNotCSI | |||
| case stateCSI: | |||
| if ch == 'm' { | |||
There was a problem hiding this comment.
What about changing the code to:
switchState:
switch ei.state {
...
case stateCSI:
switch {
case ch >= '0' && ch <= '9':
ei.csiParam = append(ei.csiParam, "")
case ch == 'm':
ei.csiParam = append(ei.csiParam, "0")
default:
return false, errCSIParseError
}
ei.state = stateParams
goto switchState
case stateParams:
...And also remove errCSINotANumber, which is not needed anymore.
|
How's the new commit look? Let me know if there's anything else I can change. |
escape.go
Outdated
| } | ||
| return false, errCSINotANumber | ||
| ei.state = stateParams | ||
| goto switchState |
There was a problem hiding this comment.
We can get rid of goto if we use fallthrough instead.
escape.go
Outdated
| @@ -85,6 +84,7 @@ func (ei *escapeInterpreter) parseOne(ch rune) (isEscape bool, err error) { | |||
| return false, errCSITooLong | |||
| } | |||
| ei.curch = ch | |||
| switchState: | |||
There was a problem hiding this comment.
Not needed anymore. See next comment.
|
Could you also rebase and squash the commits? |
According to https://en.wikipedia.org/wiki/ANSI_escape_code#CSI_codes, a zero-parameter 'CSI n m' code (for setting SGR parameters) should be treated as the same as 'CSI 0 m'. This PR changes escapeInterpreter.parseOne to support this shorthand.
|
The |
|
Merged (fast-forward) in e27f247, so I'm closing this PR manually. Thanks! |
|
Sorry, I had not read your last message before merging. Yes, keeping the |
|
No problem, thanks for the quick merge! |
According to https://en.wikipedia.org/wiki/ANSI_escape_code#CSI_codes,
a zero-parameter 'CSI n m' code (for setting SGR parameters) should be
treated as the same as 'CSI 0 m'. This PR changes
escapeInterpreter.parseOne to support this shorthand.