contain two fix and a feature#97
Conversation
sillydong
commented
Apr 20, 2018
sillydong
commented
- replace \n to space in postgresql comments
- check if schemaname exists to avoid panic
- add step support
Fix: error calling makeTable: failed to render from pipe delimited bytes: record on line 4: wrong number of fields
The old way may cause panic when the retrieved table name don’t contain schema name.
for convenience
| return s | ||
| } | ||
|
|
||
| // steps returns a live of strings of the numbers start with size of len |
There was a problem hiding this comment.
What's a "live" of strings? I don't really understand this comment at all and I can't infer from the code what it's supposed to say.
There was a problem hiding this comment.
That was a misspell. steps act like numbers return a string from start with the length of len.
|
|
||
| if c.Valid { | ||
| r.Comment = c.String | ||
| replaced := strings.Replace(c.String, "\n", " ", -1) |
There was a problem hiding this comment.
What's the rationale for this change? Might a newline not be a significant part of the comment?
There was a problem hiding this comment.
If there is \n in comment in pg, gnorm will throw error:
error calling makeTable: failed to render from pipe delimited bytes: record on line 4: wrong number of fields
This is about to fix it.
There was a problem hiding this comment.
I just ran into this issue with some of the comments in my database. Can this be merged?
There was a problem hiding this comment.
Then again, this might not be the right fix. I believe I ran into this issue when running gnorm preview on a comment containing | and ). Is there an existing issue ticket for this?
There was a problem hiding this comment.
This pr was about to fix \n in comment. I didn't meet the | or ) issue before.
There was a problem hiding this comment.
For | and ), may be needs more strings.Replace
|
Any update about this issue? Do I need to do modifies? |
3 participants
