Line breaks in GUI Data Functions break Parser

[kaiser-chris]

Hi @amtep,

I have found an issue with the GUI Data Function parser when having line breaks in it.

Example

This is valid in Victoria 3 GUIs and helps massively with readability:

onclick = "[GetScriptedGui('sgui_gate_window_execute_research').Execute(GuiScope
                .SetRoot(Country.MakeScope)
                .AddScope('technology', Technology.MakeScope)
                .AddScope('cost', MakeScopeValue(Technology.GetCost(Country.Self)))
                .End
            )]"

Errors

Below are the errors for the particular example above, but I also noticed these ones for other occurances:

• Unexpected character '.', expected ']'
• Unexpected character '.', expected ')'

warning(parse-error): quoted string not closed
   --> [MOD] gui\gate_technology_panel.gui
467 |                 onclick = "[GetScriptedGui('sgui_gate_window_execute_research').Execute(GuiScope
    |                                                                                                 ^

warning(parse-error): quoted string not closed
   --> [MOD] gui\gate_technology_panel.gui
468 |                 .SetRoot(Country.MakeScope)
    |                                            ^

warning(parse-error): quoted string not closed
   --> [MOD] gui\gate_technology_panel.gui
469 |                 .AddScope('technology', Technology.MakeScope)
    |                                                              ^

warning(parse-error): quoted string not closed
   --> [MOD] gui\gate_technology_panel.gui
470 |                 .AddScope('cost', MakeScopeValue(Technology.GetCost(Country.Self)))
    |                                                                                    ^

warning(parse-error): quoted string not closed
   --> [MOD] gui\gate_technology_panel.gui
471 |                 .End
    |     

Regards,

Chris

[amtep]

Thiscis tricky because "quoted string not closed" is a helpful warning in other contexts and I hate to lose it. I wonder if there's a way to detect that it's on purpose here

[kaiser-chris]

This is tricky because "quoted string not closed" is a helpful warning in other contexts and I hate to lose it. I wonder if there's a way to detect that it's on purpose here

Yeah the best solution would probably be to have the parser "search" for the closing quote over multiple lines.

But I don't know if that is feasible with the current design.

[amtep]

Technically it's easy :) The parser already looks for the closing quote over multiple lines. We would just have to delete the warning.

It's just that this warning is very helpful if you accidentally leave out a closing quote, so I hate to lose it. The vast majority of quoted strings are intended to be only one line. I wonder if there's a heuristic that would let the parser detect that intent.

Without this warning... Since most files have multiple quoted strings, the parser will usually find a quote mark to pair up with (namely the one starting the next quoted string), all the way to the end of the file where it will emit an unexpected eof warning. Unless you forgot TWO closing quotes, in which case it will silently mismatch them. You'll probably get other validation errors, but none of them will say "hey you forgot to close the string on this line".

[dragon-archer]

Hi, I think I have two potential solutions.

1. We can add a special detection for exactly this case. For example, when a quote mark " is directly followed by an opening bracket like [, the warning will be discarded by tiger.
2. We can add a new feature to tiger, allow using comments to adjust settings in-place. Just like clang-tidy can be suppressed by // NOLINT, we may adapt similar usage, using i.e. # tiger off to suppress errors temporarily,

I suppose solution 1 will be much easier to implement, but solution 2 will be more extensible for the future.

[kaiser-chris]

Hi, I think I have two potential solutions.

1. We can add a special detection for exactly this case. For example, when a quote mark " is directly followed by an opening bracket like [, the warning will be discarded by tiger.
2. We can add a new feature to tiger, allow using comments to adjust settings in-place. Just like clang-tidy can be suppressed by // NOLINT, we may adapt similar usage, using i.e. # tiger off to suppress errors temporarily,

I suppose solution 1 will be much easier to implement, but solution 2 will be more extensible for the future.

The second one is already there with the robust filtering system. Yes it can not be as specific as ignoring a line but still I don't think that one would be necessary.

[dragon-archer]

The second one is already there with the robust filtering system.

As far as I know, the filtering system can only be used from config file instead of any inplace think like an comment. This is useful when you want to filter out lots of warnings, but it may filter out too much and hide the real error. Meanwhile, using an inplace comment is like an explicit declaration that causes far less misuse.

[kaiser-chris]

I would like both solutions. Since one can be used for future false positives and one solves this specific issue.

[amtep]

I like solution 1.

Solution 2, lint suppression, is a big topic of its own. I was thinking of block-scoped suppression like Rust has, but line by line suppression like you suggest here would be easier to implement. It's out of scope for this issue though.