Skip to content
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

feat/docs(editorconfig): add support for spelling_language #28638

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

sus-domesticus
Copy link
Contributor

This fixes #28626.

For the other editorconfig properties it's easier to check for validity since they are either a number or part of an enum with few elements. The problem for spelling_language is that it is part of a greater enum since it's a concatenation of a ISO 639 language code and an optional ISO 3166 territory identifier.

For now the code only checks that the value provided in spelling_language is of the format [a-z][a-z] or [a-z][a-z]-[a-z][a-z] (regex), but it doesn't check whether it is a valid ISO639/ISO3166 code and thus when inputting an invalid value the spelllang option is also set to an unknown language (this results in a prompt asking you to download the unknown language and then failing and continuing if accepted).

Copy link
Member

@gpanders gpanders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good on first glance, thanks! Will give it a test whenever you mark it ready for review.

runtime/lua/editorconfig.lua Outdated Show resolved Hide resolved
runtime/lua/editorconfig.lua Outdated Show resolved Hide resolved
runtime/lua/editorconfig.lua Outdated Show resolved Hide resolved
@sus-domesticus sus-domesticus marked this pull request as ready for review May 4, 2024 15:44
@gpanders
Copy link
Member

gpanders commented May 6, 2024

Can you please also update news.txt and add a test case to editorconfig_spec.lua?

@gpanders gpanders added this to the 0.11 milestone May 6, 2024
@sus-domesticus
Copy link
Contributor Author

I encountered problems with the test_case method (in editorconfig_spec.lua).
The error is from passing { buf = 0 } to nvim_get_option_value when getting the value of spell because it's window-local not buffer-local.
error

Should I adapt test_case for the window-local spell option?

https://github.com/sus-domesticus/neovim/blob/8372a48a3564564577ca5096a0c4584f0cdcd9a8/test/functional/plugin/editorconfig_spec.lua#L14-L22

The `test_case` method presumed that all options set by editorconfig
were *buffer-local* so it called `nvim_get_option_value` with `{ buf = 0
}` even for *window-local* options such as `spell`.

Now `nvim_get_option_info2` is used to get the scope of the option. This
scope can be: "global", "win" or "buf".
@sus-domesticus
Copy link
Contributor Author

Looks like I forgot to add some parameters. It's weird because when I ran make test locally I didn't get an error.

A *GOOD* example of spelling_language is en-US. ('-' must be used as a
separator)

A *BAD* example of spelling_language is en/US or en_US.
@zeertzjq zeertzjq removed their request for review May 7, 2024 09:47
spelling_language = en

[long_spelling_language.txt]
spelling_language = en-US
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably should test something other than the default

These were changed to use something other than the default values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

editorconfig: support spelling_language property
5 participants