More robust comment string parsing#354
More robust comment string parsing#354juanjux merged 4 commits intobblfsh:masterfrom juanjux:fix_strfind
Conversation
uast/transformer/semantic.go
Outdated
|
|
||
| func (c *commentElems) Split(text string) bool { | ||
| if !strings.HasPrefix(text, c.StartToken) || !strings.HasSuffix(text, c.EndToken) { | ||
| trimmed := strings.TrimSpace(text) |
There was a problem hiding this comment.
This shouldn't happen automatically. It belongs to a separate op, or should be specified explicitly as a flag.
There was a problem hiding this comment.
Sorry, I rebased and this got fakely outdated. You mean like for example adding a new "trimText" field to commentUAST and a new op like CommentTextTrimmed?
There was a problem hiding this comment.
Yes, something like this will be nice :)
But if positions are off the node will fail the token verification for sure. So fixing positions should fix both issues (validation and trim).
There was a problem hiding this comment.
Done. In the case of Python it works because it only happens with comments on their own line that are (sometimes) indented. In the native AST those doesn't have position, unfortunately, trough in this specific case it should be probably easy to fix (and in some other cases with some heuristics).
uast/transformer/semantic.go
Outdated
| c.Prefix = text[:i] | ||
| text = text[i:] | ||
|
|
||
| if i == -1 { |
There was a problem hiding this comment.
This might be nicer:
c.Prefix = ""
if i >= 0 {
c.Prefix = text[:i]
// ...
}There was a problem hiding this comment.
Yeah, but is assigns twice in the most common case and I don't trust Golang optimizer much, anyway I also find it better and I don't think it'll hurt much.
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
dennwc
left a comment
There was a problem hiding this comment.
Few more nitpicks. Feel free to ignore if you have limited time.
| } | ||
|
|
||
| func CommentTextTrimmed(tokens [2]string, vr string) Op { | ||
| return &commentUAST{ |
There was a problem hiding this comment.
You can probably do something like
op := CommentTextTrimmed(tokens, vr).(*commentUAST)
op.doTrim = trueor add an unexported newCommentText with the third argument.
Co-Authored-By: juanjux <juanjo@juanjoalvarez.net>
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
HasPrefixandHasSuffixto avoid cases where the language can give a wrong position for the start of the comment (or only the line) like happens with Python with some comments where it gives all the whitespace before if they're on their own line and indented.Signed-off-by: Juanjo Alvarez juanjo@sourced.tech