Config parser doesn't ignore comments

[Philip Taylor]

When a config file says something like

key = value ; comment

the config parser thinks the key is associated with two values, "value" and " ; comment". (See mainlog.html, where it reports all the config values that were loaded.)

This needs to be fixed, so legitimate (comma-separated) values are kept but comments are discarded.

[Kevin Read]

I'll look into it.

[Quentin Pradet]

binaries/data/config/default.cfg is correctly parsed, and has lines like:

"hotkey.leave = Escape ;End current game or Exit."

I do not dare to use "worksforme", but I'm not sure it is really a bug. How do you reproduce it?

[Philip Taylor]

As far as I'm aware, the game still works because it only looks at the first value for each config name (in this case "Escape") and ignores the rest (the comment). Looking in mainlog.html should show the full list of values found by the config parser, which includes the comments.

[Quentin Pradet]

Indeed, the parser returns a list of values, and the two first ones are key and value. Since the code that use the parser then take value 0 and 1, the comments are ignored. Isn't that safe since the comments will always end the line?

[Philip Taylor]

The config code seems to be using more than just the first key and value, e.g. the log says:

Loaded config std::string "hotkey.camera.zoom.out" = "Minus"
Loaded config std::string "hotkey.camera.zoom.out" = "NumMinus"
Loaded config std::string "hotkey.camera.zoom.out" = " ; Zoom camera out."

so it's picking up the two legitimate values plus the comment, and maybe the comment gets ignored because it's not a valid key name (though I've not looked at the code to see what's going on). It doesn't break the game, but it's printing comments to the log as if they're values, so it's ugly and buggy and would be nice to fix :-)

[Quentin Pradet]

Okay. In fact, the parser detects comments, but still returns them as arguments.

Here is a patch fixing that. It assumes that "$rest" will always be a comment. It seems safe to assume that. The only two occurences of $rest in the code are those two line, and are used to parse .cfg files.

parser.InputTaskType("Assignment", "_$ident_= _[-$arg(_minus)]_$value_,>_[-$arg(_minus)]_$value[[;]$rest]"); parser.InputTaskType("CommentOrBlank", "_[;[$rest]]");

Feel free to do whatever you want with the patch, merge it, refuse it, modify it, etc. The licence is BSD.

[Quentin Pradet]

Patch that remove comments from the config parsing. Tests included.

[Philip Taylor]

(Changing assignment since there was no activity from obsidian in a month and I don't want that to block resolution of the issue.)

[philip]

(In [7109]) Fix #251 (Config parser doesn't ignore comments), based on patch from Cygal

[Kevin Read]

Sorry about that, guys, I must admit I got sidetracked and forgot about the issue :(

Nice work cygal!

[(none)]

Milestone Open Source Release deleted