-
Notifications
You must be signed in to change notification settings - Fork 5
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
Parser fixes #20
base: master
Are you sure you want to change the base?
Parser fixes #20
Conversation
ascii/ascii.go
Outdated
@@ -26,14 +26,15 @@ func NewASCII(dat string) (*ASCII, error) { | |||
var row []Cell | |||
chompState, fg, bg, fgs, bgs := 0, -1, -1, 0, 0 | |||
var bold, underline, italic, strikethrough bool | |||
for _, v := range dat { | |||
for i, v := range dat { | |||
switch v { | |||
case '\r': | |||
continue | |||
case '\n': | |||
bold, underline, strikethrough, italic = false, false, false, false | |||
cells = append(cells, row) | |||
chompState, fg, bg = 0, -1, -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chompState, fg, bg, fgs, bgs := 0, -1, -1, 0, 0
so it matches the initialization at the beginning of the function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in df8451a
@@ -74,8 +84,11 @@ func NewASCII(dat string) (*ASCII, error) { | |||
break | |||
} | |||
if v == ',' { | |||
chompState, bg = 2, -1 | |||
continue | |||
if len(dat) > i+1 && dat[i+1] >= '0' && dat[i+1] <= '9' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
peaking ahead like this breaks the fsm formulation; I think the handling should be in case 2
as like if v >= ... {} else if bgs == 0 { chompState = 0; break }
s := "hello \x0315,01hello" | ||
// 012345 6789 | ||
a, _ := NewASCII(s) | ||
if c := a.Get(2, 0); c == nil || c.Value != 'l' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too much repetition-- idiomatic go uses a table driven test pattern like https://github.com/chzchzchz/sitbot/blob/master/msl/grammar_test.go#L49
Struct would be like
struct {
input string
col []int
expected []Cell
}
Driver would be like
for _, tt := range tests {
a, _ := NewAscii(tt.input);
for i := range tt.col {
c := a.Get(tt.col[i], 0)
if c == nil || c != tt.expected[i] {
t.Errorf("got %+v, want %+v at %d for %q", c, tt.expected[i], tt.col[i], tt.input)
}
}
ascii/ascii.go
Outdated
@@ -60,6 +61,15 @@ func NewASCII(dat string) (*ASCII, error) { | |||
continue | |||
} | |||
case 1: | |||
if v == '\x03' { | |||
if fgs > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if fgs == 0 { bg = -1 }
fg, fgs = -1, 0
continue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 5c52340
No description provided.