wmllint++ - wmllint improved!

Discussion of all aspects of the game engine, including development of new and existing features.

Moderator: Forum Moderators

Post Reply
Groggy_Dice
Inactive Developer
Posts: 165
Joined: February 4th, 2011, 6:19 am
Contact:

wmllint++ - wmllint improved!

Post by Groggy_Dice »

I have been busy modding wmllint, adding new features and changes that I consider improvements. However, a beefed-up wmllint is of limited use while it sits on only one person's computers. Although I am still working on further enhancements, I think it's time to make it downloadable for anybody else who could use a better wmllint. And so I introduce wmllint++ - wmllint improved!
downloads of first version (outdated)
wmllint110.7z
wmllint++ for 1.10
(outdated version)
(22.6 KiB) Downloaded 472 times
wmllintdiff.7z
diff of wmllint++ versus wmllint 1.10 from svn
(outdated version)
(5.61 KiB) Downloaded 484 times
For those of you who remember the suggestions for wmllint development that I began offering a few months back, many of these features should sound familiar. However, that thread included ideas that I did not know how to implement (at least at the time). In the meantime, I've been looking up Python documentation (particularly about regular expressions) to expand my own capabilities. I also assumed that even if any of my ideas were ever adopted, they would probably only be added to the development branch. wmllint++, on the other hand, works on 1.10.

(I've also been making a few more modest improvements to wmllint 1.4, which is still used to help convert UMC from 1.0 and 1.2, because support for early WML was dropped from later versions. I'm still working on it, though.)

So just what are these purported "improvements?"

1 Various spelling corrections

I'd fixed a couple of typos that I noticed, then decided to run a spellcheck.

Code: Select all

@@ -63 +63 @@
-# You can force otherwise undeclared characters to be recogized with
+# You can force otherwise undeclared characters to be recognized with
@@ -85 +96 @@
-# You can suppress complaints about files without an initial textdoman line
+# You can suppress complaints about files without an initial textdomain line
@@ -111 +122 @@
-# spellchecker should rever flag.  If the keyword "general" is
+# spellchecker should never flag.  If the keyword "general" is
@@ -533 +545 @@ def validate_on_pop(tagstack, closer, filename, lineno):
-    # hack; some compaigns like to generate entire side declarations with
+    # hack; some campaigns like to generate entire side declarations with
@@ -753 +791 @@ def pangostrip(message):
-    "Strip Pango margup out of a string."
+    "Strip Pango markup out of a string."
@@ -1379 +1464 @@ def consistency_check():
-                    print '"%s", line %d: %s has unkown base %s' % \
+                    print '"%s", line %d: %s has unknown base %s' % \
@@ -1394 +1479 @@ def consistency_check():
-            # We have a list of all the usage types recruited at this sifficulty
+            # We have a list of all the usage types recruited at this difficulty
@@ -1946 +2049 @@ def translator(filename, mapxforms, textxform):
-        # Perform checks that are purel local.  This is an
+        # Perform checks that are purely local.  This is an
@@ -2067 +2170 @@ def inner_spellcheck(nav, value, spelldict):
-        # No match? Strip posessive suffixes and try again.
+        # No match? Strip possessive suffixes and try again.
2 Update 1.4's {LOYAL_UNIT} macro to {NAMED_LOYAL_UNIT}

wmllint doesn't transition 1.4's {LOYAL_UNIT} to {NAMED_LOYAL_UNIT}, probably because the name went to a new macro. However, a regular expression can be used to make a safe substitution. The current {LOYAL_UNIT} ends after the x,y coordinates, so if there are two more blocks of text afterwards, it must be the old macro.

When I report the substitution to stdout, I add a little message, to prevent anyone from getting the idea that they should change all {LOYAL_UNIT}s to NAMED_.

Code: Select all

@@ -1236,0 +1298,24 @@ def global_sanity_check(filename, lines):
+        # Update 1.4's {LOYAL_UNIT} macro to {NAMED_LOYAL_UNIT}, using a
+        # regular expression to distinguish it from the current {LOYAL_UNIT}.
+        if re.search(r'^[^#]*{LOYAL_UNIT +[0-9]+ +(\([^)}]+\)|"[^"}]+"|[^("][^ }]+) +([0-9]+|[^ ]*\$[^ ]*x[^ ]*) +([0-9]+|[^ ]*\$[^ ]*y[^ ]*) +[^}]+ [^}]+}', lines[i]):
+            lines[i] = lines[i].replace("{LOYAL_UNIT ", "{NAMED_LOYAL_UNIT ")
+            print '"%s", line %d: {LOYAL_UNIT -> {NAMED_LOYAL_UNIT -- 1.4 version of LOYAL_UNIT' \
+                  % (filename, i+1)
3 Auto-recognize units created by the {NAMED_*UNIT} macros

I actually accompanied this suggestion with a bit of clumsy code, but in wmllint++ I've streamlined that original code.

The "unknown 'XXX' referred to by id" message ought to be a valuable message alerting designers to misspelled names, or characters that they forgot to create or recall. Unfortunately, it doesn't recognize units created or recalled by macros, and may be the wmllint message most prone to high false positives.

wmllint allows you to add a magic comment telling it to recognize an id, but having to do more work defeats the purpose of using a macro in the first place.

wmllint++ uses a regular expression to determine the id string field, and append its contents to the list of people that wmllint recognizes as present. This may save not just a "recognize" magic comment, but a spelling magic comment, since wmllint adds "present" to the dictionary.

This code seems to be working with no false positives. It does assume that the id string is more than one character long, so theoretically if an author used a single letter or number as the id, it would not get picked up. In fact, two campaigns sometimes use blank fields for id and name - they should probably be using the non-NAMED macros. One of these campaigns creates some characters, then uses {NAMED_* for these characters in subsequent scenarios with the unit type field in unfilled quotes - presumably looking for a recall effect. I don't know if this is working WML or not, but it fails the regex for the first field. I could change '+' to '*' in the regex to catch these, but for now I'll leave it as it is.

Also, if some UMC used its own {NAMED_*UNIT} macro that didn't follow the format of the core macros, it could give a wrong result (though it would probably fail the regex).

Code: Select all

+        # Auto-recognize the people in the {NAMED_*UNIT} macros.
+        named = re.search('{NAMED_[A-Z_]*UNIT +[0-9]+ +(\([^)}]+\)|"[^"}]+"|[^("][^ }]+) +([0-9]+|[^ ]*\$[^ ]*x[^ ]*) +([0-9]+|[^ ]*\$[^ ]*y[^ ]*) +(\([^)}]+\)|"[^"}]+"|[^("][^ }]+) +[^}]+}', lines[i])
+        if named:
+            present.append(named.group(4).strip('"() '))
4 Auto-recognize units recalled by various {*RECALL*} macros

I adopted the same approach to auto-recognize characters recalled by macros. This is trickier, due to the surprising dearth of core recall macros. It may also be more controversial, as some may question whether wmllint should be interpreting unofficial, non-core macros.

One use case is straightforward. {RECALL_OR_CREATE} used to be an official macro, so it follows a set pattern amenable to regex.

Then there is the case of the basic recall macro that simply recalls a character by id. Although it may not be official, I found that UMC overwhelmingly used the obvious name: {RECALL}.

Were there any other macro names for this basic recall function? I found that one old campaign, On Crimson Wings, used RECALL_UNIT. Checking, I found that another abandoned campaign, 1.5's attempted HttH Multiplayer, also had a macro by that name, but it was different. It recalled a unit type, though the macro would fail the regex, since it had a second field for a number.

I decided to also cover Wings in my regex, even though it's just one campaign. I did tell wmllint++ to not append the result to "present" if it was in "unit_types", as a precaution against anyone using those macro names to recall by unit type instead of by id.

Personally, I think {RECALL} should be added to the core macros. It's fairly widespread, it's the de facto standard name, and I'm actually astonished that it isn't one already.

Another obvious macro function would be to recall a unit, with specific coordinates. I figured that if a macro had RECALL in its name, three fields, and numbers for its last two fields, the first field would probably be an id string. This regex did have a false positive in mainline: Hammer of Thursagan's {RECALL_VETERAN} recalled a unit type rather than an id. So I excluded the string from present.append if it was in unit_types.

But I found that there was still a false positive: Story of Wose's {RECALLR} recalled a role, not an id. One false positive might be considered acceptable. Looking further, however, I saw that despite the lack of uniformity, just three names were in use for this macro function: {RECALLXY}, {RECALL_XY}, and {RECALL_POS}. So I decided to convert from a general rule to a regex that covered those three specific cases. Edit: Actually, role= is added to present, but I decided to leave it covering just the three specific cases.

Again, I think this is a macro that should be folded into core. It is common, and making it core would standardize the name and prevent any other names from proliferating.

Theoretically, there could be a macro where the coordinates came before the id, but I found that all of the macros that fit the pattern (of two number fields followed by a final field) were actually red herrings. I also found that all macros that didn't start with RECALL (e.g., PUT_TO_RECALL_LIST) were unsuitable.

Right now, the code seems to have no false positives. Perhaps the regexes could be tightened to exclude "$" (indicating a variable rather than an id) and "=".

Code: Select all

+        # Auto-recognize people in {*RECALL*} macros. Caution is necessary due
+        # to the lack of official recall macros.
+        if re.search('^[^#]*{[^ ]*RECALL', lines[i]):
+            recalled = re.search(r'{RECALL_OR_CREATE +(\([^)}]+\)|"[^"}]+"|[^("{][^ }]+) +(\([^)}]+\)|"[^"}]+"|[^("{][^ }]+) *}', lines[i])
+            if recalled:
+                present.append(recalled.group(2).strip('"() '))
+            else:
+                recalled = re.search(r'{RECALL(_UNIT)? +(\([^)}]+\)|"[^"}]+"|[^("{][^ }]+) *}', lines[i])
+                if recalled and recalled.group(2).strip('"() ') not in unit_types:
+                    present.append(recalled.group(2).strip('"() '))
+                else:
+                    recalled = re.search(r'{RECALL(_?XY|_POS)? +(\([^)}]+\)|"[^"}]+"|[^("{][^ }]+) +([0-9]+|[^ ]*\$[^ ]*x[^ ]*) +([0-9]+|[^ ]*\$[^ ]*y[^ ]*) *}', lines[i])
+                    if recalled and recalled.group(2).strip('"() ') not in unit_types:
+                        present.append(recalled.group(2).strip('"() '))
5 Tweak the "dying words" error message for clarity

I know that the first time I ran wmllint, my reaction to the "death event" message was puzzlement. Plenty of characters in mainline speak last words when they croak, so why is wmllint complaining? So I simply ignored it.

My text is a little more verbose, but hopefully clearer in pointing out what the problem and the solution is. This is a common error in UMC, and this could in part be because the current message isn't working. (On the other hand, perhaps these designers simply aren't using wmllint.)

One downside could be that my version leads UMC authors with complex death events to simply convert to "last breath", instead of splitting them into separate "last breath" and "die" events. But those events aren't being split now.

Code: Select all

@@ -1011,2 +1049,3 @@ def global_sanity_check(filename, lines):
-                        if deathcheck and (value == filter_subject) or (value == "unit"):
-                            print '"%s", line %d: %s speaks in his/her death event' % (filename, nav.lineno+1, value)
+                        if deathcheck and ((value == filter_subject) or (value == "unit")):
+                            print '"%s", line %d: %s speaks in his/her "die" event rather than "last breath"' \
+							      % (filename, nav.lineno+1, value)
6 Drop the .mask extension exit

This code is identical to what I presented in my proposals thread.

Basically, the Wesnoth engine seems happy to use any file called up by a mask= key, whether or not it ends in .mask or has a 'usage="mask"' attribute. So I consider it excessive to turn this into a wmllint "fatal error" - probably the largest single source of wmllint crashes today. Thus, I comment out the sys.exit.

On the other hand, some people may actually care that their file has the "proper" ending and attribute. For them, I leave a softened version of the introduction's explanation and stdout message.

Code: Select all

@@ -53,2 +53,2 @@
-# Standalone terrain mask files *must* have a .mask extension on their name
-# or they'll have an incorrect usage=map generated into them.
+# If you would like wmllint to generate usage=mask instead of usage=map in your
+# mask files, you *must* name them with the .mask extension.
@@ -1745,7 +1846,8 @@ def translator(filename, mapxforms, textxform):
             elif 'mask=' in line and not (refname.endswith("}") or refname.endswith(".mask")):
                 print \
-                      '"%s", line %d: fatal error, mask file without .mask extension (%s)' \
+                      '"%s", line %d: mask file without .mask extension - wmllint will generate usage=map instead of usage=mask in the file (%s)' \
                       % (filename, lineno+1, refname)
-                sys.exit(1)
+                # We are not exiting over the .mask extension issue anymore
+                #sys.exit(1)
7 Fix a rare wmllint-crashing map_data bug

This is the same fix from my proposals topic, except that I deleted an extra space two lines down.

wmllint crashes if the value for the map_data key consists of unfilled quotation marks (""). Only Invasion of Arendia is known to have this bug. The last two scenarios are unfinished, with just some bare-bones starter code meant to be filled in later.

Code: Select all

@@ -1754,6 +1856,7 @@ def translator(filename, mapxforms, textxform):
         if map_only or (("map_data=" in line or "mask=" in line)
                         and line.count('"') in (1, 2)
+                        and line.count('""') == 0
                         and line.count("{") == 0
-                        and  line.count("}") == 0
+                        and line.count("}") == 0
                         and not within('time')):
8 Convert backslashes in file paths to frontslashes

Some designers, out of ignorance or habit, use backslashes in their paths. However, backslashes can't be replaced indiscriminately, because they're in use as escapes (or bridge terrain).

My regex looks for backslashes in a string that ends with a file type. As a practical matter, EVERY instance in the wild seems to involve png, except for one comment in 1.2's Adventure_of_Soul_Keeper.cfg referring to a .mo file - which ironically is not one of the covered file types. Still, I included several common file types.

Besides reporting the substitution to stdout, I warn in ALL CAPS against backslashes. Some people may think it's too much.

This code does not cover directory paths - those that don't end with a file. However, I've only run across one example ("{~campaigns\A_Tale_of_Life_and_Death\scenarios}"), so it's probably not worth trying to hunt down with wmllint.

Code: Select all

@@ -1153,0 +1202,13 @@ def global_sanity_check(filename, lines):
+        # Deal with a few Windows-specific problems for the sake of cross-
+        # platform harmony. First, the use of backslashes in file paths.
+        while re.search(r'^[^#]*\\[^ ={}"]*\.(png|ogg|wav|gif|jpe?g|map|mask|cfg)\b', lines[i], flags=re.IGNORECASE):
+            backslash = re.search(r'(.*?)([^ ={}"]*\\[^ ={}"]*\.)(png|ogg|wav|gif|jpe?g|map|mask|cfg)(\b.*)', lines[i], flags=re.IGNORECASE)
+            lines[i] = backslash.group(1) + backslash.group(2).replace("\\","/") + backslash.group(3) + backslash.group(4) + '\n'
+            print '"%s", line %d: %s -> %s -- PATHS SHOULD USE THE FRONTSLASH (/)' \
+                  % (filename, i+1, backslash.group(2) + backslash.group(3), backslash.group(2).replace("\\","/") + backslash.group(3))
9 Strip userdata/ from paths

This was one of my suggested ideas, but I didn't know how to accomplish it with Python.

The misguided authors who put userdata/ in their paths cause problems not just for non-Windows users, but fellow Windows users who chose to put userdata in My Games. wmllint++ removes this mistake.

Of course, YOU know better than to add userdata/ to paths, right? If you're porting, however, you may run into this fairly common mistake of inexperienced UMC authors.

My regex is solicitous towards a set of add-ons in 1.4 that use "userdata/campaigns" instead of "userdata/data/campaigns" in their paths. The stdout includes another all-caps admonition against "userdata/".

Code: Select all

+        # Then get rid of the 'userdata/' headache.
+        while re.search(r'^[^#]*user(data/)?data/[ac]', lines[i]):
+            userdata = re.search(r'(.*?)((\.\./)?user(data/)?)(data/[ac][^/]*/?)(.*)', lines[i])
+            lines[i] = userdata.group(1) + userdata.group(5) + userdata.group(6) + '\n'
+            print '"%s", line %d: %s -> %s -- DO NOT PREFIX PATHS WITH userdata/' \
+                  % (filename, i+1, userdata.group(2) + userdata.group(5), userdata.group(5))
10 Add new 1.10 portraits to the mix of orc warlord portrait replacements

This is basically the same as my earlier proposal, though the code has been rearranged a bit.

Wesnoth transitioned in 1.8 from the five old-style orcish warlord portraits, but there were only three available replacements, requiring two to be doubled up.

wmllint++ adds two of the three new 1.10 orc portraits to the replacements, adding more variety to the portraits used for orcish leaders. Although this mod is too late for the mainline campaigns and other campaigns already converted, there are many unported earlier campaigns.

This is one way wmllint could be modified, reflecting my partiality for the grunt-5 and grunt-6 portraits. You could change the particular portraits (and their order) based on your own taste.

Code: Select all

@@ -497,6 +508,4 @@ linechanges = (
         # Changes after 1.8rc1
         ("portraits/orcs/warlord.png", "portraits/orcs/transparent/warlord.png"),
-        ("portraits/orcs/warlord2.png","portraits/orcs/transparent/warlord.png"),
-        ("portraits/orcs/warlord3.png","portraits/orcs/transparent/grunt-2.png"),
         ("portraits/orcs/warlord4.png","portraits/orcs/transparent/grunt-2.png"),
         ("portraits/orcs/warlord5.png","portraits/orcs/transparent/grunt-3.png"),
@@ -522,0 +532,3 @@ linechanges = (
+        # Changes during 1.9.x
+        ("portraits/orcs/warlord2.png","portraits/orcs/transparent/grunt-5.png"),
+        ("portraits/orcs/warlord3.png","portraits/orcs/transparent/grunt-6.png"),
11 Restore the usage class check, while creating a magic comment for non-standard classes

The "unknown usage class" message got removed during 1.9. The rationale (offered by ai0867) was, "Don't make wmllint check if the usage type matches a standard one. Non-standard ones work just fine".

I disagree with this decision. Custom usage classes are relatively rare, while the check caught real errors: typos, bogus classes, using unit types instead of usage types. Other wmllint checks have higher rates of false positives.

How then to deal with non-standard usage classes? After restoring the check, I made the list of usage types appendable, and created a magic comment, "wmllint: usagetype", for specifying new classes. This comment is comma-separable, and usagetype can even be pluralized. Unleash the stonegazers!

Code: Select all

@@ -78,0 +80,10 @@
+# If your recruitment patterns include any usage classes besides the standard
+# recruitable classes, you can tell wmllint about them with the magic comment
+# "wmllint: usagetype".  If you have more than one, this comment is
+# comma-separable, and can even be pluralized, for example:
+#    wmllint: usagetypes stonegazer, statue
+# This comment should be added in a file that wmllint checks before the
+# non-standard usage class appears in a scenario, preferably the _main.cfg.
+# (Adding it to the file of a unit generally won't work, because wmllint
+# checks the directory's /units subfolder after /scenarios, alphabetically.)
+#
@@ -630,0 +643,26 @@ notepairs = [
+# This is a list of the standard mainline recruitable usage types. (Two other
+# usage types, null and transport, are not normally recruitable.) Additional
+# usage classes can be appended with the magic comment, "#wmllint: usagetype".
+usage_types = ["scout", "fighter", "mixed fighter", "archer", "healer"]
+
@@ -1037,0 +1077,5 @@ def global_sanity_check(filename, lines):
+        # Magic comment for adding non-standard usage types
+        m = re.search('# *wmllint: usagetypes? +(.*)', lines[i])
+        if m:
+            for newusage in m.group(1).split(","):
+                usage_types.append(newusage.strip())
@@ -1095,0 +1140,4 @@ def global_sanity_check(filename, lines):
+                    for utype in recruitment_pattern[ifdef_stack[-1]][1]:
+                        if not utype in usage_types:
+                            print '"%s", line %d: unknown usage class %s' \
+                                  % (filename, i+1, utype)
12 Convert old UMC paths from data/campaigns to data/add-ons

This is one of the features I requested from the developers, now I've implemented it myself!

The reason this feature hasn't been activated before is that it would "clobber mainline" campaigns, which still use the data/campaigns path. However, I was able to make wmllint++ update UMC paths, while still being safe for mainline.

First, I defined a list of mainline campaigns. When wmllint++ encounters a line with a data/campaigns/ path, it checks all iterations against the list of mainline campaigns. Only if there is a non-match does wmllint convert the line.

This code is a little bit more elaborate than it might otherwise be, because I want wmllint++ to be able to accurately handle a hypothetical case where a line contains both a UMC and a mainline path, even though I don't know of any actual example in the wild. So the conversion tests each "data/campaigns/<name>" instance, then changes any mainline strings so the while statement can proceed. If there were any mixed lines, the altered mainline string is changed back to "data/campaigns/".

Code: Select all

+# This is a list of mainline campaigns, used to convert UMC from
+# "data/campaigns" to "data/add-ons" while not clobbering mainline.
+mainline = ("An_Orcish_Incursion",
+            "Dead_Water",
+            "Delfadors_Memoirs",
+            "Descent_Into_Darkness",
+            "Eastern_Invasion",
+            "Heir_To_The_Throne",
+            "Legend_of_Wesmere",
+            "Liberty",
+            "Northern_Rebirth",
+            "Sceptre_of_Fire",
+            "Son_Of_The_Black_Eye",
+            "The_Hammer_of_Thursagan",
+            "The_Rise_Of_Wesnoth",
+            "The_South_Guard",
+            "tutorial",
+            "Two_Brothers",
+            "Under_the_Burning_Suns",
+            )
+
@@ -1594,2 +1679,2 @@ def hack_syntax(filename, lines):
-        # The trouble with this transformation is that it's only right for UMC;
-        # it clobbers mainline.
+        # This code has been superceded, but keeping it around in case wmllint
+        # ever wants to do anything with [textdomain] or [binary_path].
@@ -1603,10 +1688,26 @@ def hack_syntax(filename, lines):
             if "[/textdomain]" in lines[i]:
                 in_textdomain = False
-            if in_binary_path or in_textdomain:
-                lines[i] = re.sub(r"data/campaigns", r"data/add_ons", lines[i])
         # This is done on every line
         if "campaigns/" in lines[i]:
             lines[i] = lines[i].replace("{~campaigns/", "{~add-ons/")
             lines[i] = lines[i].replace("{@campaigns/", "{~add-ons/")
+            # Convert UMC to data/add-ons without clobbering mainline
+            cleanup_main = False
+            for camp in re.finditer("data/campaigns/([\w'-]+)", lines[i]):
+                campaign = camp.group(1)
+                if campaign not in mainline:
+                    # Considers the possibility that a line may have both UMC and mainline paths
+                    while re.search(r"data/campaigns/[\w'-]+", lines[i]):
+                        dc = re.search(r"(.*?data/)campaigns/([\w'-]+)(.*)", lines[i])
+                        if dc.group(2) not in mainline:
+                            lines[i] = dc.group(1) + 'add-ons/' + dc.group(2) + dc.group(3) + '\n'
+                            print '"%s", line %d: data/campaigns/%s -> data/add-ons/%s'\
+                            %(filename, i+1, dc.group(2), dc.group(2))
+                        else:
+                            lines[i] = dc.group(1) + 'xxmainlinexx/' + dc.group(2) + dc.group(3) + '\n'
+                            cleanup_main = True
+            # Converts back mainline paths in the case of mixed UMC/mainline lines
+            if cleanup_main:
+                lines[i] = lines[i].replace('xxmainlinexx/', 'campaigns/')
13 Backport a couple of bugfixes from trunk

For the most part, I haven't sifted through the changes in trunk, to see which are related to WML changes in 1.11 and which might be useful backports. However, I spotted two bugfixes that carry back to 1.10.

r55044 by ai0867: Don't strip underscores from keys when trying to remove a translation mark

Code: Select all

@@ -1312 +1397 @@ def global_sanity_check(filename, lines):
-                    lines[i] = lines[i].replace("_", "", 1)
+                    lines[i] = prefix + value.replace("_", "", 1) + comment + '\n'
r55045 by anonymissimus: fix # wmllint: deathcheck off directive being ignored in some cases
Last edited by Groggy_Dice on February 19th, 2013, 10:08 am, edited 2 times in total.
Ports:
Prudence (Josh Roby) | By the Sword (monochromatic) | The Eight of Cembulad (Lintana~ & WYRMY)
Resources:
UMC Timeline (Dec) | List of Unported UMC (Dec) | wmllint++ (Feb)
Groggy_Dice
Inactive Developer
Posts: 165
Joined: February 4th, 2011, 6:19 am
Contact:

Re: wmllint++ - wmllint improved!

Post by Groggy_Dice »

I've come out of the weekend with a new version. Just one new feature, but I've made wmllint++ speedier.

The new feature: converting "~add-ons" in textdomain/binary paths to "data/add-ons". It's not one of the most common mistakes, but there was that in_textdomain and in_binary_path code lying around, going unused...

As far as speed: wmllint++ was noticeably more sluggish than the regular wmllint. However, since the search for the 1.4 {LOYAL_UNIT}, auto-recognition of {NAMED*}, and auto-recognition of {RECALL*} all involve macros, I put them inside a filter for macros in the line, rather than having all lines being searched three times. I also put the backslash and userdata/ replacement inside simple if tests, rather than going directly to the complicated regexes. After these changes, wmllint++ is just seconds slower than wmllint, in exchange for expanded functionality.

I've done some more testing over the weekend, and found that auto-recognition was sometimes crashing the enchant spell checker, when the id was empty after being stripped. For example, in scenario 9 of Descent Into Darkness, there are some macros in the format, {NAMED_NOTRAIT_UNIT ...... ("") (_ "Guardian")}. The regex for the id field requires at least one character inside the parentheses, but the quotes satisfy that. But the strip() takes out both the parentheses and the quotes, leaving an empty result. When the ids in "present" are passed to the spell checker, the empty string crashes enchant.

So I added a filter to make sure there was at least one non-stripped character before the result was appended to present. I also found that Wesnoth will indeed recall a previously created unit with these macros, even with an empty unit type field. So I changed the regex to allow for an empty unit type.
Attachments
wmllintdiff.7z
diff of wmllint++ and wmllint for 1.10
(5.71 KiB) Downloaded 484 times
wmllint110.7z
wmllint++ for 1.10
(22.69 KiB) Downloaded 467 times
Ports:
Prudence (Josh Roby) | By the Sword (monochromatic) | The Eight of Cembulad (Lintana~ & WYRMY)
Resources:
UMC Timeline (Dec) | List of Unported UMC (Dec) | wmllint++ (Feb)
User avatar
esr
Retired Developer
Posts: 228
Joined: November 26th, 2006, 6:40 pm
Location: Pennsylvania, USA
Contact:

Re: wmllint++ - wmllint improved!

Post by esr »

wmllint's author speaking:

Congratulations - you are now the maintainer of wmllint. Do you have commit access to the repo?
Anonymissimus
Inactive Developer
Posts: 2461
Joined: August 15th, 2008, 8:46 pm
Location: Germany

Re: wmllint++ - wmllint improved!

Post by Anonymissimus »

esr wrote:wmllint's author speaking:

Congratulations - you are now the maintainer of wmllint. Do you have commit access to the repo?
Hm, could you please be not so quick with this ?
While this looks very promising, he should first post some patch for *trunk* at gna http://gna.org/patch/?group=wesnoth for you, AI etc to approve and apply.
(I need to check that it keeps working on windows I suppose...)
projects (BfW 1.12):
A Simple Campaign: campaign draft for wml startersPlan Your Advancements: mp mod
The Earth's Gut: sp campaignSettlers of Wesnoth: mp scenarioWesnoth Lua Pack: lua tags and utils
updated to 1.8 and handed over: A Gryphon's Tale: sp campaign
User avatar
Iris
Site Administrator
Posts: 6796
Joined: November 14th, 2006, 5:54 pm
Location: Chile
Contact:

Re: wmllint++ - wmllint improved!

Post by Iris »

Anonymissimus wrote:While this looks very promising, he should first post some patch for *trunk* at gna http://gna.org/patch/?group=wesnoth for you, AI etc to approve and apply.
(I need to check that it keeps working on windows I suppose...)
More like a series of tidy and self-contained patches, in fact, to make it possible for us to review what each modification really does.

Incidentally, here’s a handy link to our patch submission guidelines.
Author of the unofficial UtBS sequels Invasion from the Unknown and After the Storm.
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: wmllint++ - wmllint improved!

Post by Boucman »

In this particular case, and since it's going for wmllint maintainer, it's fine to post patches for wmllint, no need to learn c++

I'm probably stating the obvious but it's always better to state it.

your current wmllint++ could probably be cut into reviewable patches and that would do the trick
Fight key loggers: write some perl using vim
Groggy_Dice
Inactive Developer
Posts: 165
Joined: February 4th, 2011, 6:19 am
Contact:

Re: wmllint++ - wmllint improved!

Post by Groggy_Dice »

My first two patches have been up a couple of days, simple ones to "test the waters." I'm afraid of messing up the line numbers if I go any further, however. Maybe I just haven't figured out the Subversion way, but from what I can tell, you can't easily make a set of patches reflecting multiple local changes. There's no local repository to build on step by step, you can only diff/patch against the last official version you have.

In order to get multiple discrete patches, it looks like I'll have to do something like this:

First, wait for the patches I've already submitted to be decided on, then sync with the repository.

Then, find the bottom-most set of changes to make, and add them to the updated file. Create a patch.

Then, find the next-bottom-most revision, add it, and create a patch. Delete the first change from the patch to make a discrete patch for the second feature.

Find the next-bottom-most change, make it, delete the previous changes from the patch. Repeat till I get to the top-most change.

Submit the patches in order (and hope that they are applied in order).

Once they are approved, repeat for another set of patches.

Perhaps I'm missing something that would make this much easier. Am I?
Ports:
Prudence (Josh Roby) | By the Sword (monochromatic) | The Eight of Cembulad (Lintana~ & WYRMY)
Resources:
UMC Timeline (Dec) | List of Unported UMC (Dec) | wmllint++ (Feb)
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Re: wmllint++ - wmllint improved!

Post by Boucman »

i'm not sure I understand, are your patches submitted ? if yes i'll ping esr so he reviews them
Fight key loggers: write some perl using vim
AI
Developer
Posts: 2396
Joined: January 31st, 2008, 8:38 pm

Re: wmllint++ - wmllint improved!

Post by AI »

[patch]3751[/patch] and [patch]3752[/patch].
Post Reply