Include gdscript warning name in the warning message. #40155
Conversation
modules/gdscript/gdscript.cpp
Outdated
| } | ||
| ERR_FAIL_V_MSG(String(), "Invalid GDScript warning code: " + get_name_from_code(code) + "."); | ||
| return msg + " (" + get_name() + ")"; |
There was a problem hiding this comment.
Indentation issue.
I'd suggest using square brackets ([UNUSED_ARGUMENT]) instead of parentheses, as it's what is used for compiler warnings (at least GCC and Clang's).
There was a problem hiding this comment.
I'm fine either way, but FWIW pylint uses parens, and gdscript seems most similar to python:
[rcorre@midova tmp]$ pylint example.py
************* Module example
example.py:1:0: C0305: Trailing newlines (trailing-newlines)
Anyone familiar with python will immediately understand that "the thing in parens" is what they can put in the ignore comment.
There was a problem hiding this comment.
Alright, I'm also fine either way :) I guess it depends what compiler warnings folk are most used too (C/C++ vs Python).
|
Please squash the commits. |
Occasionally you want to ignore a warning with a `warning-ignore` comment, and you have to go into the settings to look up what the actual name of the warning is. This patch appends the warning name to the end of the warning so you know what string to use to ignore it, similar to other linters like pylint. For example ``` "The signal 'blah' is declared but never emitted."; ``` is now ``` "The signal 'blah' is declared but never emitted. (UNUSED_SIGNAL)"; ```
Calinou
added
cherrypick:3.x
|
I don't really see the point of this, since you can just click "Ignore" in the warning itself. |
|
@YeldhamDev you can only click "ignore" if you're using the builtin editor. This is useful for folks using an external editor (which can now show warnings thanks to LSP support 🎉 ) |
|
Oh, didn't think about that! The change is fine then. |
|
Thanks! |
|
Cherry-picked for 3.2.3. |
akien-mga
removed
the
cherrypick:3.x
|
Now that I'm testing, I'm not sure this was a good change, as it adds redundant information which was already included in the warning: I guess your motivation for adding this is that you want it visible in the debugger even without expanding the warning details, i.e. when it's like this: But as it stands it adds redundant info for the other two cases. |
|
The LSP calls get_message to build the warning message it sends, so no external editor will see the warning name without this patch. |
|
So I suppose the "proper" fix would be to replicate https://github.com/rcorre/godot/blob/0fe60f84241663a053cde622567c669f2b6093c9/modules/gdscript/gdscript.cpp#L624 in the LSP. |
I can open a proposal if desired, but this is just a pretty small
quality-of-life thing.
Occasionally you want to ignore a warning with a
warning-ignorecomment, and you have to go into the settings to look up what the
actual name of the warning is. This patch appends the warning name to
the end of the warning so you know what string to use to ignore it,
similar to other linters like pylint.
For example
is now