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

Textidote: translate localized &spelllang for textidote #1805

Closed
wants to merge 1 commit into from

Conversation

bratekarate
Copy link
Contributor

Maps &spelllang to the format that textidote understands. Allows the use of localized languages such as en_gb. Handles all languages that are supported by textidote as of the current version.

@lervag
Copy link
Owner

lervag commented Sep 26, 2020

Thanks, this looks reasonable. I agree it could be useful or interesting to allow a config dictionary to specify the desired language when it is ambivalent.

@bratekarate
Copy link
Contributor Author

Thank you for including this feature! Also, good to see a better way of handling the varying length of &spelllang with only one matchlist execution in your commit Still learning vimscript.

@lervag
Copy link
Owner

lervag commented Sep 26, 2020

My pleasure. Let me know if you think we need more possibilities for customisation here.

@oblitum
Copy link

oblitum commented Oct 1, 2020

Vim supports multiple spell languages in &spelllang, I myself have it set to set spelllang=en,pt_br. Isn't that a problem because it seems code here just assumes one language, not two or more.

Maybe it should avoid drawing from &spelllang and simply provide a custom configuration for the tool as it doesn't support that and the user may be working on, say, pt_br text, but current check just grabs en or something.

@lervag
Copy link
Owner

lervag commented Oct 1, 2020

Good point. Could you open a new issue for it, and I can try to make this general purpose enough to work for all grammar/language parsers?

@bratekarate
Copy link
Contributor Author

True, I did not think of multiple &spelllang either. But I don't agree that it should be left out all together -- multiple languages looks rather like an edge case to me, and it is easy enough to just use the first language by default.

A variable to override &spelllang would basically suffice to cover all edge cases. I think it would be rather inconvenient for most users to manage the state of two, basically redundant variables. At least for me it would be reduntant, as I usually switch the spelllang instead of using multiple. If I write in German, I only want German words to be recognized.

On another note, I tested the compiler with multiple languages and found a little bug that causes e.g. en_ca to turn into enCA (missing underscore). However, this is not related to multiple spelllangs, but rather a general bug for localized languages. Other than this bug, the only thing that makes get_textidote_lang fail for multiple languages is the hard check on en_gb.

I used a modified version of the function in this PR for ale. The code of the ale linter can possibly be improved, but it may be helpful as it would cover the exact use case that is talked about here.

@oblitum
Copy link

oblitum commented Oct 1, 2020

@bratekarate from my experience in vim world (a decade or so), the setting isn't much edge, I've seen lots of vimmers setting two languages in &spelllang.

@bratekarate
Copy link
Contributor Author

@oblitum You are probably right and more experienced than I am with vim. I still think that picking the first spelllang in the list is a reasonable default. Even if more than 50% of the users were to use multiple spelllangs, the others without multiple languages still profit from the default. The vimmers with multiple languages don't lose anything, having the config variable anyway. Even if you use multiple: Isn't it convenient to use the primary language and override the variable only if needed in buffer scope?

Anyway, I'm also ok with configuring another variable for my spellchecker if that's the consensus. Not a biggie.

@oblitum
Copy link

oblitum commented Oct 1, 2020

@bratekarate I don't disagree with that, an override/fallback scheme as you say is just fine, it just not need to be hard-coded, my previous comment was just to point that it isn't much an edge case.

@lervag
Copy link
Owner

lervag commented Oct 1, 2020

It seems @oblitum has also suggested a solution for this. I hope it's OK that I continue this discussion at #1811 after considering the PR (I'm working on something else right now (#1799), so I won't promise it will happen immediately, sorry).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants