Add columns to Language Server for external editors

[voylin]

This PR is to have column support for the language server when using external text editors. Full-filling this TODO mentioned here:

godot/modules/gdscript/gdscript_parser.cpp

Line 237 in 40cf5a0

// TODO: Improve positional information.

This should not break anything and just add's the functionality to get more feedback of what part of the line contains the warning or the error.

Without this PR:

With this PR:

[voylin]

Does anything still need to happen for this PR to be merged or just waiting for a review? @dalexeev

[voylin]

I re-based onto the master branch and did the requested changes. There's only the one review conversation about the loop which I'm not 100% certain with of what to do.

[voylin]

Added the changes requested and everything works as expected:

[HolonProduction]

I'm wondering whether we should adjust the test runner to print the full ranges. Right now it only prints the start line for parser errors and the start and end lines for warnings. But the reason for that is probably that no more information was available at the time.

On the other hand this would mean touching all GDScript tests 🤔

[voylin]

I'm wondering whether we should adjust the test runner to print the full ranges. Right now it only prints the start line for parser errors and the start and end lines for warnings. But the reason for that is probably that no more information was available at the time.

On the other hand this would mean touching all GDScript tests 🤔

Want to give me a go at it and add it to this PR?

[voylin]

@HolonProduction I felt that the error/warning messages with line and columns is part of this PR so I went ahead and implemented the changes in the script and checked/adjusted all of the .out files. Was quite tedious work. ^^"

I'm not certain if the error's from the parser are what is desired, but I'll leave that upon your feedback to decide how to write those.

I've added it as a separate commit in case you prefer that this is done through a separate PR, but it does feel like part of this commit. Let me know if I should merge both commits.

[dalexeev]

I don't think this is a good idea. It's not visually clear, and it's unlikely anyone will monitor the column numbers. It just creates unnecessary noise and makes it difficult to edit tests.

I think the correctness of source regions should be ensured by other measures. There's no point in adding full positional information to every test. Especially in analyzer tests, since errors/warnings are generated on AST nodes, so positional information will always be correct if the previous steps (tokenization and parsing) are bug-free and the correct target node for the error/warning is selected.

Please revert these changes. Even if we decide such an improvement is necessary, it's beyond the scope of this PR and should be moved to a separate PR.

[voylin]

Please revert these changes. Even if we decide such an improvement is necessary, it's beyond the scope of this PR and should be moved to a separate PR.

Okay, changed it back ;)

[HolonProduction]

Code makes sense to me. Tested this with VSCode and it seems to work fine.

[HolonProduction]

Might need an adjustment to the commit message though, which contains the entire squash noise

[voylin]

Might need an adjustment to the commit message though, which contains the entire squash noise

Cleaned it up. ;)

[voylin]

@dalexeev Thanks for pointing out my mistakes, I had forgotten to order the start_* and end_* together after changing the lines. My mistake, sorry. I'll be more careful! ^^"

[dalexeev]

Looks good to me. 👍 Thank you for your patience!