Pass information to ParserError in order to show user a better error message #52
Conversation
glessard
force-pushed
the
duplicative-flags
branch
from
32d5370 to
6477202
Compare
|
This conflicts with the changes in #21, where the internals of (specifically, I used the getter for |
natecook1000
left a comment
natecook1000
left a comment
There was a problem hiding this comment.
Looking good! Just a couple small requests.
| func testDuplicateFlags() { | ||
| AssertErrorMessage(Options.self, ["--list", "--bool", "-s"], "Value to be set with flag \"-s\" had already been set with flag \"--list\"") | ||
| AssertErrorMessage(Options.self, ["-cbl"], "Value to be set with flag \"l\" in \"-cbl\" had already been set with flag \"c\" in \"-cbl\"") | ||
| AssertErrorMessage(Options.self, ["-bc", "--stats", "-l"], "Value to be set with flag \"--stats\" had already been set with flag \"c\" in \"-bc\"") |
There was a problem hiding this comment.
Nice work, these are great messages!
| func duplicateExclusiveValues(previous: InputOrigin, duplicate: InputOrigin, arguments: [String]) -> String? { | ||
| func elementString(_ origin: InputOrigin, _ arguments: [String]) -> String? { | ||
| guard case .argumentIndex(let split) = origin.elements.first else { return nil } | ||
| var argument = "\"\(arguments[split.inputIndex.rawValue])\"" |
There was a problem hiding this comment.
We're using single quotes around user input in the other messages, can you do the same here?
There was a problem hiding this comment.
I had failed to notice. Done!
| return "flag \(argument)" | ||
| } | ||
|
|
||
| let dupeString = elementString(duplicate, arguments) ?? "position \(duplicate)" |
There was a problem hiding this comment.
Can you add to-do comment here to about updating this when we support environment values? It's worth noting that the RHS of these coalescing operators won't ever get hit.
glessard
force-pushed
the
duplicative-flags
branch
from
af08a46 to
4eccca6
Compare
|
I reinstated some of what had been removed in 6b3a0a1, because it was what I'd used to peek in and extract the necessary info. If there's another method, I can use that, but I haven't seen it. |
glessard
force-pushed
the
duplicative-flags
branch
from
4eccca6 to
7c642ec
Compare
2 participants
This is an attempt to fix #49
Questions about this proposed fix:
I touched every place where "duplicateExclusiveValues" appears, so it's not clear that error case would have another use than this one.