Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wmllint: asserts if [unit_type]'s advances_to is before the id #4102

Closed
stevecotton opened this issue Jun 3, 2019 · 6 comments
Closed

wmllint: asserts if [unit_type]'s advances_to is before the id #4102

stevecotton opened this issue Jun 3, 2019 · 6 comments
Labels
Enhancement Issues that are requests for new features or changes to existing ones. Low Priority Issues that will cause no meaningful problems if left unaddressed. WML Tools Issues involving WML maintenance tools.

Comments

@stevecotton
Copy link
Contributor

The code checking the [unit_type] data asserts if it meets the advances_to before the id:

wesnoth/data/tools/wmllint

Lines 1520 to 1521 in 23ab654

elif key == "advances_to":
assert(unit_id or base_unit)

Ageless Era's preprocessed.cfg has all the keys of [unit_type] sorted in to alphabetic order, so this triggers on it.

@jostephd
Copy link
Member

jostephd commented Jun 3, 2019

wmllint should really use wmlparser3 or something so it doesn't depend on order of lines.

@stevecotton
Copy link
Contributor Author

I'm wondering if it's a tool to fix, or a tool to just leave as-is.

@ProditorMagnus
Copy link
Contributor

I did something similar which I use to validate the ageless preprocessed WML https://github.com/ProditorMagnus/WML_tree_tools/blob/master/py_files/find_patterns.py

@jostephd
Copy link
Member

jostephd commented Jun 5, 2019

@stevecotton I don't know about wmllint specifically, but in general, time spent on writing lint tools is time well spent. We can make the effort incrementally too, port the wmllint checks to wmlparser3 one by one...

@sevu sevu added Enhancement Issues that are requests for new features or changes to existing ones. Low Priority Issues that will cause no meaningful problems if left unaddressed. WML Tools Issues involving WML maintenance tools. labels Jun 6, 2019
@soliton-
Copy link
Member

soliton- commented Jun 7, 2019

In general it's difficult to use a proper parser for a tool like wmllint that for example should upgrade old syntax or check macro usage where a parser would perhaps error out or not give you the needed info because macros are already expanded. Using comments as directives as is done now will likely also be very difficult to support.

Certainly some checks like this one would benefit from a proper parser though...

@jostephd
Copy link
Member

jostephd commented Jun 8, 2019

Using comments as directives as is done now will likely also be very difficult to support.

I think it's possible. The parser would need to remember a line number for every syntax element, and the line number that every comment appeared on, but it's not like we have to invent the wheel.

Agree with your remark about old syntax / macro usage.

Elvish-Hunter added a commit that referenced this issue Dec 3, 2020
This was supposed to be a part of the fix for #4102, but for some reason it never got committed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Issues that are requests for new features or changes to existing ones. Low Priority Issues that will cause no meaningful problems if left unaddressed. WML Tools Issues involving WML maintenance tools.
Projects
None yet
Development

No branches or pull requests

5 participants