This allows faulty .pbl files to be reported to the user in a clear
fashion instead of presenting them as a "network communication error"
(which is very wrong, seeing as how these issues only concern local
files).
The actual error is both logged in stderr and presented in the UI as in
the following example:
> Error
>
> A file with add-on publishing information could not be read.
>
> File: /home/shadowm/.wesnoth-1.11/data/add-ons/After_the_Storm.pbl
> Error message: Unexpected characters after variable name (expected , or =), value '' at <unknown>:24
This introduces a new minimal exception object to encapsulate some
relevant information (the file path and the config::error message) so it
can be propagated to the manager UI code correctly.
This is the first step is ensuring faulty .pbl files are not reported
during connection to the add-ons server as a "network communication
error".
The convention that [textdomain] uses "/translations" is strongly established, and I can't think of a legitimate reason for an add-on not to be following it.
The binary path check is a crude test. The names that took hold for menu image directories are "/public" and variations of "/external*", so we look for those strings. It does not catch the worst case of all - when all binaries are outside the campaign define, not just a set-aside directory.
I first thought of these checks while brainstorming ways to use the in_textdomain and in_binary_path code in hack_syntax(). However, realizing that these checks did not really hack any syntax, I wanted to find someplace in the sanity checks where the code would fit. I finally found it.
While there are many places that wmllint assumes that "#" begins a comment, from Vultraz's description it sounded like parse_attribute was being used to find the attribute's value, and then a string_strip was done on the value. So I looked for the direct cause in wmltools.
Again, the solution is to look for whitespace to precede the hashsign.
Incidentally, I don't know why the original code had the first "where -= 1", then had value and comment go from [:where+1] and [where+1:].
I had a hunch that the hash would be the problem, because I realized that wmllint usually just looked for '#' to figure out if a line contained a comment, and that this also matched pango color coding. My wmllint backslash/userdata commits used this same re.split match to prevent this false positive.
(It won't prevent all unintended matches, e.g. "Guard #4", but I don't think that can be helped.)
The problem came, I believe, when the old line interacted with the block ten lines down:
elif eligible.count('"') % 2:
dostrip = not dostrip
The value's closing quote was on the other side of the split.
This thing was introduced in 492efa7b4bf687cada5d125d2fc4ab1c9ab097db
(from SVN r35965) due to a misapplication of a SVN property manipulation
command. I don't know what it is, but since it's part of the
decommissioned stats.w.o codebase and with the wrong filename to begin
with, I doubt anyone will miss it much.
This is necessary so files with similar names which are actually
versioned and important aren't matched by these patterns, otherwise
only intended to match build output files.
Here's the rationale for these additions:
* There is so much focus on wmllint's role in conversion, that many people may not think of it as a validator also (I didn't). So often, stumped authors ask in the forums about problems that would have been fixed or pointed out if they'd run wmllint. I want to encourage awareness of wmllint as a validator.
* Folded a line to fit normal 80-width CLI.
* Help contained no mention of this rather redundant option.
* How many people don't realize that ESR's long introduction is there?
* Some users may not understand why they're being dumped back to wmllint's help.
I used "inconsistency" for the actual variable name, because "known" seems more likely to be accidentally reused.
I pondered whether to allow the scenario check to go forward, but decided to just make a clean break.
Note that this does not prevent any of the information-gathering for the consistency check, just the check itself.
Why would you want to use this option? Of course, you should run the consistency check at some point. But if you simply want to recheck if you've fixed all the bugs in your campaign, you might not want to have wmllint slog through data/core again.
According to the introduction, stringfreeze does *not* suppress the warning, and the code bears this out.
I wonder how often this option is actually used.
The main way I know of for getting into this situation is hitting
the recursion maximum within pump(), and if that happens you probably
have runaway WML. So it should be better to move on instead of
continuing to process events.
(Normally, events raised within an event handler are pumped before
that event handler exits, so would not be affected by this change.)
I think the old behavior is an artifact of the old implementation,
but I'm making this a separate commit in case removing this causes
problems (easier to diagnose, especially if I'm not the one diagnosing).
game_events::pump() now records the events in the event queue at
the time it is called. This isolates those events from any that
might be fired within one of the events (e.g. with [fire_event]).
For an example of what was happening, see bug #21031.
Fixes bug #21031.
