-
Notifications
You must be signed in to change notification settings - Fork 63
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
Cannot define methodmap properties in single line #845
Comments
I'm open to allowing one-liners like this, as long as there is only a getter OR setter (not both), and the function body is at most one statement. But regardless macros shouldn't be used this way. Or ideally, at all. It is poor practice. |
Curious as to why there should be any restriction on one-lining whatsoever, surely it's not the compilers job to enforce code styling (loose indentation being an exception because it can indicate a mistake)? In my case I've got a methodmap that has 100+ properties (for working with a large config file) that are literally all the same 4 lines except for stringmap keys and types. Almost all of the properties are in groups of 2 or 3 for different related values like value + enabled state + chat notification. |
It's definitely within a compiler's job description to prevent unwanted practices. Your use case is a classic example of where automatic code generation is much more preferable. Have a script (bash, Python, whatever) dump out the methodmap for you and you'll never have to touch it again. |
@sirdigbot If you need help with code generation, I have a library called sptools which can lex + parse SP but also has a working code formatter as well. |
That's fair, though I don't really think code style fits perfectly into that category. Particularly I think rules over whitespacing is crossing onto the wrong side of the line (lookin at you, python). If you're sticking with it anyway, there should be a clearer explanation in the error message. Maybe something like
This is a good idea, especially for anything large that frequently changes. But for a chunk of code that is essentially not going to change at all it's more effort than actually just writing out the properties manually. And another issue is that parts of the methodmap aren't generated, or have to have some sort of unique behaviour compared to how it's implemented in the config file (e.g. Admin flag strings are converted to an int, some properties need to clamp values between 0 and 255, names of properties are not identical to their user-friendly name in the config or are condensed/split into separate values). If the generation is only going to include part of the methodmap then it adds a different kind of error-prone build step where you need to remember to properly merge the output with the non-generated bits of code. Frankly I think the very-obviously-working-otherwise-the-whole-thing-explodes-and-also-it's-just-part-of-the-regular-compilation macros would be a much cleaner alternative. |
I can't argue that the way macros are often used otherwise is terrible, though (lookin at you, LoopClients) |
+1 If you want object-like properties, you need to use methodmaps backed by StringMap. |
Possible duplicate of #844 but i can't understand what is being said there so ¯\_(ツ)_/¯
As the title says, methodmap properties can't be defined on a single line without an error (specifically the last
}
cant be on the same line as the get/set function).Happens in both 1.10 and 1.11.
My specific use case is using a macro to define properties in a methodmap. Which I know is probably not a use case that should be supported. But regardless, its kinda weird to error here anyway.
The text was updated successfully, but these errors were encountered: