-
Notifications
You must be signed in to change notification settings - Fork 40
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 JSON hell, and enable more kinds of hell in the future #137
Add JSON hell, and enable more kinds of hell in the future #137
Conversation
brilliant |
return n, err | ||
switch cType { | ||
case PlainText: | ||
if _, err = bw.WriteString("# Chapter 1\n"); err != nil { |
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.
note: this may actually make my comment about the break
s irrelevant as well as you wouldn't even need a switch
more nitpicking: we can probably just define a map, something like
var appleTree = map[ContentType]string{HTML: "<html>\n<body>\n", foo: "bar", } // etc
// [...]
var (
forbiddenFruit string
ok bool
)
if forbiddenFruit, ok = appleTree[cType]; !ok {
panic("this is a tomato")
}
if n, err = bw.WriteString(forbiddenFruit); err != nil {
return n, err
}
// [...]
something like this is probably a tidier solution, obviously you don't need to be as silly as i'm being here in terms of var naming
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.
actually i set this up because i needed the markovmap to also escape newlines and quotation marks, but i never got to that part :/
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.
actually i set this up because i needed the markovmap to also escape newlines and quotation marks, but i never got to that part :/
a bit confused as to what you mean, couldn't one just use "`" (backtick) characters to wrap the string values of the map?
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.
im pretty sure that is not valid in JSON
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.
the json is inside the backticks, not the other way around
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.
no, i mean that everything sent from the markov is escaped, like newlines become literal \n
s, and quotation marks become \"
s, or else an iterative parser can stop processing immediately
im not sure how to add a wrapper that will edit it on the fly
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.
i'm clearly missing something, let me take a deeper look at this
@ShadowJonathan i changed the target branch to dev, if you wanna resolve the conflicts we can yolo merge it |
b652344
to
54e492c
Compare
@yunginnanet should work now |
Closes #136
This also adds a generic kind of system so that more kinds of hell can be added in the future