LuaLS / lua-language-server

A language server that offers Lua language support - programmed in Lua
https://luals.github.io
MIT License
3.34k stars 318 forks source link

Mass renaming proposal #1404

Open carsakiller opened 2 years ago

carsakiller commented 2 years ago

There are a number of settings, diagnostics, and more that have unclear names that I think can be improved. This issue serves to propose new names for these items so that they can all be tracked in one place and implemented at once to minimize the number of breaking releases that would need to be released.

Settings

Diagnostics

I imagine there is a good reason abiguity-1 is named how it is 😄 I see they all have at least 1 hyphen so I guess that is a limitation with how they are parsed?

Syntax Errors

Luckily these can probably be changed with very little impact on the user and most people likely would not have any settings related to these.

newfield-call and newline-call are not easy to understand but I can't think of anything that does a better job 🤔

I don't even know what these do: unexpect-efunc-name, unexpect-lfunc-name


Please let me know what you think and feel free to suggest other names.

sumneko commented 2 years ago

unexpect-efunc-name:

local x = function x() end

unexpect-lfunc-name

local function x:t() end
flrgh commented 2 years ago

:wave:

workspace.userThirdParty -> workspace.envEmulations

This one seems like a step in a very specific direction, and I'm not sure I agree with it.

Most of the existing third-party annotation libraries are very framework-y in nature, so referring to them as environments makes perfect sense. However, we also have at least one that is purely a standalone library though (lfs).

In my ideal scenario there would be support for both frameworks and many more libraries, which can be detected at runtime when the server sees require "{{name_of_library}}" in a file (I am personally working on type annotations for Penlight and luasocket). Calling the setting envEmulations makes the framework use case a little more intuitive, but it makes the library use case much less intuitive.

Maybe this specific feature needs its own discussion though. It's possible that supporting standalone libraries was never a goal for this feature anyhow.

flrgh commented 2 years ago

Thanks @carsakiller for writing this up and starting the discussion

Diagnostics & Syntax Errors

No opinions about these other than keeping them consistent with one another.

I also think it would be well worth the effort to add some code to help users transition to the new names by detecting the old ones and offering to migrate and save the settings for them. Users spend a lot of time tuning and tweaking their settings, and nothing is more frustrating than when a backwards-incompatible change invalidates hours of hard work (this is the sort of thing that has made me give up on using several otherwise-great editor plugins in the past).

carsakiller commented 2 years ago

Thanks for the response, it is great bouncing ideas around 🙂

Third Party Directories

My impression was always that third party directories were meant as a more in-depth solution that would not just offer definitions, but also be able to emulate a target environment as closely as possible. Whether you actually modify the environment through more than just definition files or not, I think that name is most fitting for the function of that feature. Using the third party directories to just automatically load definition files is still modifying the simulated environment, after all. I can see what you mean though, as if you are just trying to automatically apply definition files, environment emulation sounds a little too advanced/off-topic. Perhaps environment configuration would be better as both just definition files and full frameworks configure the environment.

hover.previewFields -> hover.peekFields

This suggestion is definitely one of the more insignificant ones as it is really just replacing a word with a synonym. My intention is, seeing as VS Code uses peek in a few areas already that have a similar function, I was hoping it would be more obvious how it will look. It is absolutely not a necessary change, I just figured that if we are already renaming so many things, it would be best to do it now even if it only offered marginal improvements at best.

runtime.special -> runtime.specialVars

I have personally never used, or had a need, to use this setting. Regardless, I do like your suggestion. I think we can do one better though and just do runtime.aliases.

Syntax Errors

I noticed that a lot of the syntax errors start with err-, but not all. I personally don't see much of a purpose of prefixing syntax errors with something that makes it more obvious that you are receiving a syntax error instead of a diagnostic warning, seeing as their purpose is the same. So maybe we remove all the err- prefixes?

carsakiller commented 2 years ago

I have edited this issue to add proposals for removing all (at least the ones I could find) err- prefixes for syntax errors.

I also changed environmentEmulation to environmentConfig - how does that sound @flrgh?

FlashHit commented 2 years ago

please ping me if anything gets changed, so I can adjust my workspace settings in time.

carsakiller commented 2 years ago

If there is going to be this many things changing, I think it will be something that everyone gets notified of in advance.

carsakiller commented 1 year ago

@sumneko, unless the intention is to perform the rename in a backwards-compatible way (which would be nice, just sounds difficult), semver recommends that the changes result in a major version bump (4.0.0).

sumneko commented 1 year ago

It is not difficult to keep backwards-compatible

hinell commented 11 months ago

I find increasingly confusing the following two options for LLS config:

The runtime.path is easily confused for lua or neovim runtime libraries, not current project's one "runtime" lua files whatever that means. I propose to soft-deprecate runtime.path in favor of workspace.input or workspace.files.

And the workspace.library in favor of workspace.dependencies or workspace.libraries like it was suggested above.