Bug #768
Problem with "//" in Ini parser
Status: | Approved | Start date: | 04/26/2014 | |
---|---|---|---|---|
Priority: | Normal | Due date: | ||
Assignee: | Jan Dolinár | % Done: | 0% | |
Category: | Core | Spent time: | - | |
Target version: | - |
Description
Attached patch fixes troubles with "//" in configuration variables, mentioned in this topic: http://www.ultimatepp.org/forums/index.php?t=msg&goto=43042
The bug can be reproduced by using ini file containing something like this:
@replace-env var=file://$HOME/test
History
#1 Updated by Jan Dolinár almost 11 years ago
- File deleted (
ini.patch)
#3 Updated by Miroslav Fidler almost 11 years ago
- Status changed from Patch ready to Ready for QA
- Assignee changed from Miroslav Fidler to Jan Dolinár
In the end, I have decided to refactor the code a bit more, please check... (note: autotest/LoadIniStream should be testing this)
#4 Updated by Jan Dolinár almost 11 years ago
- File ini2.patch
added
Miroslav Fidler wrote:
In the end, I have decided to refactor the code a bit more, please check... (note: autotest/LoadIniStream should be testing this)
It works well, only thing I'd disagree with you is the skipping of spaces. I think there should be either "while(*s <= ' ') s++;" or nothing, so that all whitespace characters are treated the same way. But I'd vote to skip this entirely, it is not very common to add space between $ and variable name. Unless you know about some reason I didn't think about :)
Also, while we are at this, I'd also like to add solution to one more related problem I hit recently: There is no way to write things like "$USER_backup", because entire string would be interpreted as variables name. The solution is to allow writing "${USER}_backup", as it is done in shell. Also, it allows to use non-standard characters in variables (it is allowed on Windows and not directly prohibited by POSIX). The attached patch does exactly this. Autotest is modified accordingly.
#5 Updated by Miroslav Fidler almost 11 years ago
- Status changed from Ready for QA to Approved
Thanks!
#6 Updated by Miroslav Fidler almost 11 years ago
P.S.: Next time please change "Assigned to"... :)