Bug #768

Problem with "//" in Ini parser

Added by Jan Dolinár over 7 years ago. Updated over 7 years ago.

Status:ApprovedStart date:04/26/2014
Priority:NormalDue date:
Assignee:Jan Dolinár% Done:

0%

Category:CoreSpent 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

ini.patch Magnifier (588 Bytes) Jan Dolinár, 04/26/2014 02:08 PM

ini2.patch Magnifier (1.38 KB) Jan Dolinár, 04/28/2014 06:48 PM

History

#1 Updated by Jan Dolinár over 7 years ago

  • File deleted (ini.patch)

#2 Updated by Jan Dolinár over 7 years ago

Updated the patch...

#3 Updated by Miroslav Fidler over 7 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 over 7 years ago

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 over 7 years ago

  • Status changed from Ready for QA to Approved

Thanks!

#6 Updated by Miroslav Fidler over 7 years ago

P.S.: Next time please change "Assigned to"... :)

Also available in: Atom PDF