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

Support language tool language auto detection #103

Closed
oblitum opened this issue Sep 6, 2021 · 17 comments
Closed

Support language tool language auto detection #103

oblitum opened this issue Sep 6, 2021 · 17 comments
Assignees
Labels
1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature 3-fixed Issue resolution: Issue has been fixed on the develop branch
Milestone

Comments

@oblitum
Copy link

oblitum commented Sep 6, 2021

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
Using actual real-world examples that explain why you and many other users would benefit from the feature increases the request's chances of being implemented.

LanguageTool have language auto detection:

  -adl, --autoDetect       auto-detect the language of the input text - note this will not detect
                           variants like 'English (US)', so you will not get spell checking for
                           languages with variants

It would be great if ltex-ls provided a means to use that instead of only supporting hardcoded language.

Describe the solution you'd like
A clear and concise description of what you want to happen.

I wish some setting, no design in specific.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

None, currently I'm just configuring it manually.

Additional context
Add any other context or screenshots about the feature request here.

sylvainhalle/textidote#151

@oblitum oblitum added the 1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature label Sep 6, 2021
@oblitum
Copy link
Author

oblitum commented Sep 6, 2021

I'd like to take the opportunity to say thank you for this project, it's great! I'd like also to share that I have forked vscode-ltex into coc-ltex, to get mostly all features in Vim or NeoVim. It's a minimal fork to reduce maintenance when syncing to upstream, which I plan doing from time to time.

@valentjn valentjn self-assigned this Sep 6, 2021
@valentjn valentjn added the 3-fixed Issue resolution: Issue has been fixed on the develop branch label Sep 6, 2021
@valentjn
Copy link
Owner

valentjn commented Sep 6, 2021

Thanks for the feature request. I did look at this before, but the fact that variants couldn't be detected, which resulted in having no spelling errors, drove me away. I'll add to the docs that it's not recommended to use this.

Regarding coc-ltex, is it possible to use some if clauses to have one repo for both? Or would that be too ugly? The differences don't seem to be too large. I don't use Neovim, but if it's possible to test in an automated way that I don't accidentally add new stuff that's only available in VS Code, then this might be feasible.

@oblitum
Copy link
Author

oblitum commented Sep 6, 2021

the fact that variants couldn't be detected, which resulted in having no spelling errors, drove me away. I'll add to the docs that it's not recommended to use this.

Yeah, that's not great indeed, although I like having basic auto detection at hand, even though missing variant specific.

Regarding coc-ltex, is it possible to use some branching to have one repo for both?

I'm not sure. I believe it should, as you can see, I'm just trying to apply the strictly needed changes for most of what I could infer of features to work on coc.nvim's side. I can say it's working pretty well, even for code comments, just working out of the box:

gif-2021-09-06-141742

I am in fact trying to manage some kind of branching in my fork, I'm leaving the develop branch there intact, while working on the default coc branch solely, and I'm applying some kind of versioning that embedded both the base vscode extension version, and the coc variant, hence 11.0.0-0.1.coc (based on vscode-ltex 11.0.0, with 0.1 coc changes on top).

@oblitum
Copy link
Author

oblitum commented Sep 6, 2021

This is my first coc.nvim extension, and it's a fork 99.999999% of the work is yours. I'm not into developing it that much as I'm short of time, so, my stance with it at the moment is just to keep it up with minimal changes as far as it works, but I can't atm go much further into providing custom tests, etc.

@oblitum
Copy link
Author

oblitum commented Sep 6, 2021

"if clauses" might not be a great route because often coc.nvim forks of vscode extensions end up providing coc/vim/nvim specific features that don't make sense in VScode, meaning, they can be their own beast, and often they are. It would work ideally when the developers actually care for providing support for both, including eventual specific features.

valentjn added a commit to valentjn/vscode-ltex that referenced this issue Sep 8, 2021
@valentjn valentjn added this to the 14.0.0 milestone Oct 14, 2021
valentjn added a commit to valentjn/vscode-ltex that referenced this issue Oct 14, 2021
@valentjn
Copy link
Owner

Feature released in 14.0.0.

Regarding coc.nvim, vscode-ltex 12.0.0 now supports VS Code as well as coc.nvim. I had looked at the diff and it didn't seem large to me, more akin to a different compilation target. The advantages are that vscode-ltex's CI automatically builds the coc.nvim version as well (i.e., incompatibilities are spotted earlier) and the coc.nvim version benefits from earlier updates, feature improvements as well as security/bug fixes (e.g., there was a tar issue in the coc.nvim version). What's not done currently is publishing the coc.nvim version to npm, as I didn't want to interfere with the existing module.

@oblitum
Copy link
Author

oblitum commented Oct 14, 2021

This is really great. I'm a bit unsure how to proceed now, it seems that means one could just :CocInstall vscode-ltex and ignore about the existence of coc-ltex? If that's the case, I may drop the module from npm. If it's better to keep it (in case that doesn't work or given all coc extensions are coc-something), I may look how to transfer ownership, if you prefer that.

(cc @chemzqm / @fannheyward for advice)

@oblitum
Copy link
Author

oblitum commented Oct 14, 2021

Ah, vscode-ltex isn't on npmjs.com, so :CocInstall vscode-ltex isn't expected to work.

I can manage owners for the current coc-ltex package, but I didn't find an user with your GitHub username there.

@fannheyward
Copy link

fannheyward commented Oct 15, 2021

coc.nvim supports install extensions from Github like this: :CocInstall https://github.com/valentjn/vscode-ltex@develop. coc.nvim will download the tar.gz from Github and run npm install.

I've tried this but: vscode-ltex needs to run python3 tools/patchForTarget.py first before npm install and compile build.

This means we can't use CocInstall to install it, but vim-plug can:

Plug 'valentjn/vscode-ltex', {'branch': 'develop', 'do': 'python3 tools/patchForTarget.py --target coc.nvim && npm install && npm run coc.nvim:prepublish'}

@valentjn
Copy link
Owner

valentjn commented Oct 15, 2021

TypeScript and npm don't support conditional building, that's why the patching pre-processing step is necessary. As only one target can be built at any time, the best solution is probably having a separate npm module as before. This way, the workflow doesn't change for existing users, and users don't have to worry about the patching step, since the CI would take care of this. I wouldn't use vscode-ltex as name for the module, as that would be pretty confusing (plus, this would prevent other targets that vscode-ltex might support in the future; maybe I'll publish the VS Code version as well). If it's fine with you, I can publish to coc-ltex. I created an account on npmjs.com.

@oblitum
Copy link
Author

oblitum commented Oct 15, 2021

@valentjn I've added you as owner. It seems at the moment something is on hold for it to take effect as npm owner list coc-ltex only displays myself Already took effect.

@valentjn
Copy link
Owner

@oblitum Thank you. Just published vscode-ltex/coc-ltex 12.1.0 (*) via a new GitHub Actions job, and LTEX seems to work for me after installing with :CocInstall coc-ltex.

(*) As I like to view coc.nvim as just another target like VS Code, vscode-ltex and coc-ltex will always have the same latest version.

@oblitum
Copy link
Author

oblitum commented Oct 16, 2021

@valentjn really nice, just upgraded, working without issues. Gonna drop my repositories now.

@valentjn
Copy link
Owner

@oblitum It wouldn't have been possible without your input. Thanks again.

@oblitum

This comment has been minimized.

@oblitum

This comment has been minimized.

@valentjn
Copy link
Owner

Please open new issues for any bugs (or feature requests) to help keep things concise, either here for server issues or over at valentjn/vscode-ltex for client issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-feature-request ✨ Issue type: Request for a desirable, nice-to-have feature 3-fixed Issue resolution: Issue has been fixed on the develop branch
Projects
None yet
Development

No branches or pull requests

3 participants