Skip to content

Change to luarc is causing some issues #831

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

Closed
Cussa opened this issue Nov 29, 2021 · 11 comments
Closed

Change to luarc is causing some issues #831

Cussa opened this issue Nov 29, 2021 · 11 comments
Labels
bug Something isn't working question User has a question

Comments

@Cussa
Copy link

Cussa commented Nov 29, 2021

Describe the bug
The change from settings.json to the new .luarc.json is causing some issues. As all of them are related to this change I will keep all of them in this same ticket. If you think that it would be better to split them in separated tickets, please, just tell me and I will do that.

First Bug: configuration is not showing anymore in the settings page

To Reproduce
Steps to reproduce the behavior:

  1. Configure the .luarc.json file, including a globals, for example
  2. Open the settings
  3. Search for lua
  4. Verify the global

Expected behavior
Show the configurations now coming from .luarc.json

This was working in the 2.3.7, when the configuration was still in the settings.

Second bug: intellisense in .luarc.json doesn't follow the example file

The example file uses a structured declaration, while the intellisense uses a single-line declaration. It seems that there are some issues with that.

Third bug: quick fixes are not working.

When finding some issue, VS Code shows the option to do a quick fix. However, the fix is not going to any file, and doesn't works as expected. The error persists.

Environment (please complete the following information):

  • OS: Ubuntu
  • WSL 2 and Docker (tested in both)
  • Client: VSCode
@sumneko sumneko added the bug Something isn't working label Dec 1, 2021
@sumneko
Copy link
Collaborator

sumneko commented Dec 1, 2021

  1. .luarc.json is different from .vscode/setting.json. .vscode/setting.json is only for VSCode so you can find them in VSCode setting GUI.
  2. Both structured and single-line declaration are valid.
  3. What is the error message?

@sumneko sumneko added Info Needed More information is required question User has a question labels Dec 1, 2021
@Cussa
Copy link
Author

Cussa commented Dec 1, 2021

@sumneko can you confirm if in the VS Code we should continue to use the vscode settings file?

Because we saw some issues and thought that we should move to .luarc.json. But if I got your answer correctly, we should stick to the settings file.

@Cussa
Copy link
Author

Cussa commented Dec 1, 2021

We managed to create an example that shows the issue.

We created a workspace that has the following structure:

image

print_file.lua

print("hello")

global_problem.lua

image

If we try to use the quick fix, it will try to include that in the Workspace configuration. However, as workspaces are not exactly shared, we prefer to use the file configuration (settings.json or the most recent .luarc.json). As the settings doesn't really work for workspaces, we went with the luarc solution.

So, we created the .luarc.json with just the schema.

{
    "$schema": "https://raw.githubusercontent.com/sumneko/vscode-lua/master/setting/schema.json",
}

Using the intelisense, we can add the diagnostics. This makes the IGNORED_GLOBAL work.
image

But we want to add more information: the library and the max preload file size. When we did that, the IGNORED_GLOBAL stopped to work again.
image

Doing some tests, we saw that if we used the "structured" way for the diagnostics global, it started to work again.
image

Another thing that we saw is that even in the single-line specification, if we remove one of the workspace definitions, it start to work again.
only "workspace.library"
image

only "workspace.maxPreload"
image

We thought the issue could be related to two settings that have the same "prefix". So, we added the ignoreDir. It continued to have the issue.
image

However, if we remove the library and keep the ignoreDir and the maxPreload, it works properly!
image

Hopefully this will give you enough information to maybe figure out the issue.

If you need any more information, I will be happy to help.

sumneko added a commit that referenced this issue Dec 2, 2021
@sumneko
Copy link
Collaborator

sumneko commented Dec 2, 2021

The second bug should be resolved.
The reason is that we use multiple keys to represent the same setting, and I will read the configuration from three places and merge the configuration. But I erroneously merged the settings before the key normalizing, causing different expressions to be overwritten in an unexpected order. Fortunately, the pairs function of the lua runtime I used will not be randomized (but the order is still unexpect), so the problem can be reproduced stably.

@sumneko
Copy link
Collaborator

sumneko commented Dec 2, 2021

I guess the third bug is:

{
    "diagnostics.globals": [
        "GLOBAL1"
    ]
}

When you use quickfix to add global GLOBAL2, I will modify the .luarc.json to:

{
    "Lua.diagnostics.globals": [
        "GLOBAL1",
        "GLOBAL2"
    ],
    "diagnostics.globals": [
        "GLOBAL1"
    ]
}

This caused me to still read the old settings.
Later, I may need to use a special library to process .luarc.json (I just simply use a general json library now, it can't handle the order).

@Cussa
Copy link
Author

Cussa commented Dec 2, 2021

Yes, the third bug is related to what you mention and it's still happening. The other bug if fixed.

@Cussa
Copy link
Author

Cussa commented Jan 10, 2022

Hello @sumneko . Do we have any change related to this? It seems that the .luarc.json file is not working anymore in VSCode.

image

If I try to use the quick fix, it is creating a settings.json file in the .vscode folder. As we have now the luarc.json file option, that's better for us.

@sumneko
Copy link
Collaborator

sumneko commented Jan 10, 2022

The multi-workspace should be supported in next version, you could checkout the WIP version from https://github.com./sumneko/lua-language-server/actions/runs/1677515565, check if it works.

@carsakiller
Copy link
Collaborator

@Cussa has your issue been resolved?

@Stanzilla
Copy link

Using quickfix still renames diagnostics.globals to Lua.diagnostics.globals at least, not sure if that is actually an issue though?

@sumneko
Copy link
Collaborator

sumneko commented Nov 16, 2022

The main problem of this issue has not been solved, but the priority is not high.

@sumneko sumneko removed the Info Needed More information is required label Nov 16, 2022
sumneko added a commit that referenced this issue Dec 1, 2022
add tests
@sumneko sumneko closed this as completed in 6999dbb Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question User has a question
Projects
None yet
Development

No branches or pull requests

4 participants