Fix parsing python multiline string #299
Conversation
gandarez
force-pushed
the
bugfix/python-multiline
branch
from
f79afbf
to
22eeae9
Compare
|
@unknwon any update here? |
|
Thanks for the PR! Will take a look when time permits. |
| // Determine indent size and line prefix. | ||
| currentIndentSize := len(peekMatches[1]) | ||
| if indentSize < 1 { | ||
| indentSize = currentIndentSize | ||
| p.debug("readPythonMultilines: indent size is %d", indentSize) | ||
| } | ||
|
|
||
| // Make sure each line is indented at least as far as first line. | ||
| if currentIndentSize < indentSize { | ||
| p.debug("readPythonMultilines: end of value, current indent: %d, expected indent: %d, line: %q", currentIndentSize, indentSize, line) | ||
| return line, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
What is the rationale of deleting these lines? Without these, it seems we can't determine indent size for the multiline value?
There was a problem hiding this comment.
The main idea here is to not bypass any miss indentation from the source which means we're looking for break lines and not for "malformed" multiline values. Having it in the logic makes the first line to always determine the other's indentation.
There was a problem hiding this comment.
The main idea here is to not bypass any miss indentation from the source which means we're looking for break lines and not for "malformed" multiline values. Having it in the logic makes the first line to always determine the other's indentation.
What is definition of a "break line", is it a line that failed to be matched with pythonMultiline.FindStringSubmatch?
There was a problem hiding this comment.
All look good to me, just want to confirm the impact of deleting these lines.
unknwon
added
status: needs feedback
|
Hi, any update on this? |
From the history of the comments in @unknwon's review, it seems he's waiting for some clarifications from your side on the deleted lines. |
|
The detailed explanatation for this issue was first reported here. In a nutshell it's removing the original indentation when saving. |
|
@gandarez LGTM! Could you fix the conflicts so CI can be kicked off? |
gandarez
force-pushed
the
bugfix/python-multiline
branch
from
22eeae9
to
cdc3c1c
Compare
Codecov Report
@@ Coverage Diff @@
## main #299 +/- ##
==========================================
+ Coverage 88.13% 88.28% +0.15%
==========================================
Files 9 9
Lines 1357 1349 -8
==========================================
- Hits 1196 1191 -5
+ Misses 98 96 -2
+ Partials 63 62 -1 |
|
https://github.com/go-ini/ini/releases/tag/v1.65.0 has been tagged for this merge. |
What problem should be fixed?
This PR fixes parsing Python multiline strings to keep the original indentation instead of removing all leading spaces. It breaks some compatibility with legacy Python parsers as them fail to parse multiline strings that are malformed. This issue was first reported here.
Have you added test cases to catch the problem?
Yes