Expose getLocationLineAndColumn() as a public API #1282
Conversation
| }; | ||
|
|
||
| JSONTEST_FIXTURE_LOCAL(GetLocationLineAndColumnTest, test) { | ||
| const std::string example = "line 1\nline 2\r\nline 3\rline 4"; |
There was a problem hiding this comment.
This is not enough testing. There's only one input here.
There should be an exhaustive loop checking the results of every position.
This input only has one kind of newline.
| @@ -392,6 +392,12 @@ bool JSON_API parseFromStream(CharReader::Factory const&, IStream&, Value* root, | |||
| */ | |||
| JSON_API IStream& operator>>(IStream&, Value&); | |||
|
|
|||
| /** Get the line and column of a character within a string. | |||
There was a problem hiding this comment.
It feels like a different general function that has nothing to do with JSON.
This isn't part of Reader's API, so I'm not sure it belongs in reader.h.
The out parameters should be a returned struct rather than int references, and the location should be a size_t, as it's encoding an array offset.
The documentation should be more complete about whether the line and column are 1 or 0 based, how it handles newlines, and how it handles unicode multibyte chars. I mean is 八 counted as one character or are all of its utf-8 bytes counted? If you're doing line and column, then you're implicitly talking about display position and it's a can of worms.
But really I have to question its role in the API. It doesn't really fit in.
|
I created this PR, along with #1281, because I would like to get more detailed line/column information from both the error (right now it only returns a string) and the values in case of success. This PR is not needed very badly, because clients that want to convert a #1281 is more important to me, because it exposes information that I can't get without parsing the returned error string. |
The description field of this PR and #1281 are both empty. Can you write descriptions for them so we can understand the motivation? |
OK, done. |
|
Yes, #1281 is independent of this. |
|
Closing this due to inactivity. Feel free to reopen if you are able to address feedback. |
getLocationLineAndColumn()is an internal API used by the error string code, but clients which useJson::Value::getOffsetStart()andJson::Value::getOffsetLimit()may want to usegetLocationLineAndColumn()to turn the offset into a line and column. Expose the function as an external API.