-
Notifications
You must be signed in to change notification settings - Fork 4
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 heuristics for ignoring numbers #26
Conversation
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.
SGTM. Do you intend to keep adding more and more flags? Maybe it's just me, but I'll probably never end up using the majority of those flags, and instead report issues when the defaults are too aggressive.
stdout 'main.go:20:44: "1e-347" is misspelled in comment' | ||
stdout 'main.go:21:44: "1e-346" is misspelled in comment' | ||
stdout 'main.go:22:44: "1e-345" is misspelled in comment' | ||
stdout 'main.go:23:44: "1e-344" is misspelled in comment' |
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.
A couple more that come to mind: the hexadecimal \xa3
as in Go strings, and human-friendly "thousands" quantities like 10k
or 300k
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.
That is a plan in #8 (not the words, but they should get split by the camel splitter).
} | ||
s.Init(fset.AddFile("", fset.Base(), len(word)), []byte(word), eh, 0) | ||
_, tok, lit := s.Scan() | ||
return !errored && lit == word && (tok == token.INT || tok == token.FLOAT || tok == token.IMAG) |
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.
Hmm. I was initially going to suggest https://pkg.go.dev/go/parser#ParseExpr, but that's potentially more expensive if e.g. the word has special characters in it.
Since you only care about int/float/imag, you could also consider direct calls to the three strconv APIs. That might be simpler, though then you'd have to try each of the three in sequence.
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 issue is that strconv doesn't handle binary exponent floats, otherwise that's what I would have done.
main.go
Outdated
|
||
var ( | ||
fset = token.NewFileSet() | ||
s scanner.Scanner |
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'd definitely avoid a global named s
:)
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 intend to have some kind of configuration that can be placed in the repo, I've just been adding the flags to start with as procrastination. |
Updates #8
/cc @mvdan