-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
linter/markdown: adds support for languatool #2155
Conversation
The propaganda is so strong on those issues that I'm trying it now. |
Location list position for me seems off, using example from #1516. How did it go for you? |
Columns are right, lines are not. |
In fact, I used LanguageTool from https://github.com/rhysd/vim-grammarous and forgot it was LanguageTool based :D |
Can you give me an example file, so I can try to reproduce the issue? |
I've opened a markdown file with the contents of example in issue #1516: #1516 (comment) |
I get 8 errors, but ALE just highlighted the error on last line, all errors are in that line, which is wrong. Clicking on location list errors, it goes to the right column an error is, but doesn't jump to the affected line. |
Are you sure, that you inserted newline? |
Yes, I formatted with |
That is weird. I can reproduce the issue, but when I add newlines with |
This looks like an issues with languagetool. It reports issues in lines that are not even on the screen... |
|
Exactly, the |
Everything gets correctly highlighted without the parameter now removed from pull. |
I wished ALE linters were better at highlighting words... It just highlights first char in most. |
Because most compilers or linters only output a position instead of a text range ? I am using https://github.com/rhysd/vim-grammarous for LanguageTool right now. It can highlight a range of chars. |
Okay I'll remove the param! |
It's a bit of extra work, but, the tool does highlight entire pieces of text while this addition just highlights a single char, so, could that be added? If not, I think it's to be born with an issue open on that. Related: #2125 |
Possibly just counting the amount of ^^^^^^ in the empty line and using it as the number of chars to highlight will do a good job. See result above. |
I think vim-grammarous simply cares to process what I requested above. See #2125. |
alright, should work just fine! |
I've never seen a commit stream with that many typos, are you fine @wahrwolf? |
I am currently working on stuff for my university and am little low on sleep ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good to me. Add some tests for the linter command and executable, and for the new error handler. See :help ale-development
.
Merry Christmas!
bump. |
I have made linters for ALE too, everything worked fine. But it required tests and I encountered some vader (?? If I remember correctly) integration problems on my laptop, so I give up. the corpse of my PR is still there: The hardest part is not writing the linter, but using vader. |
See |
Oh i forgotten completly this Ticket... Going to fix it this week though.
Ty for the reminder
Am So., 27. Jan. 2019, 16:59 hat w0rp <[email protected]>
geschrieben:
… See :help ale-development-linter-tests for information on adding the
tests required for linter commands and executables.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2155 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ALOQw_apwvkoKZXkMTHu141l0ESNQoCPks5vHczsgaJpZM4ZdZSm>
.
|
Is it possible (probably with more work) to use this with Latex files? |
Heh, I just wrote a very simple linter for Neo4j's Cypher, which is working by the way. I wished to contribute but the tests part of the process look quite annoying for someone not familiar with neither Vader nor the ALE specific test setup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good. List the tool as being supported in README.md
and in doc/ale.txt
.
bump 😴 |
@wahrwolf , will you make this final move ?? |
now on to it! (i have the night and some coffee ready to go ;D )
|
@w0rp lets try this :D |
Cheers! 🍻 |
🚨 🚨 🚨 Sadly this broke mail/markdown/text/etc when languagetool is present. On file write I get some java GUIs popping up due to |
I think is the only issue, the missing |
Maybe the |
@fszymanski why? I see no reason for that. |
It's configurable. I think |
Cannot be used on Windows: let ale_linters.markdown = ['languagetool']
let g:ale_languagetool_executable = 'd:/software/LanguageTool/LanguageTool-4.4/languagetool.cmd' while
I can see the java process hangs in the background, and ALEInfo returns: The java process will never finish until I kill it manually, any clue for this ?? |
As I mentioned previously, there's a bug even on Linux, the file is not being passed, there's a diff --git a/autoload/ale/handlers/languagetool.vim b/autoload/ale/handlers/languagetool.vim
index aaad4c9..10e049d 100644
--- a/autoload/ale/handlers/languagetool.vim
+++ b/autoload/ale/handlers/languagetool.vim
@@ -10,7 +10,7 @@ endfunction
function! ale#handlers#languagetool#GetCommand(buffer) abort
let l:executable = ale#handlers#languagetool#GetExecutable(a:buffer)
- return ale#Escape(l:executable) . ' --autoDetect '
+ return ale#Escape(l:executable) . ' --autoDetect %s'
endfunction
function! ale#handlers#languagetool#HandleOutput(buffer, lines) abort |
will this PR ever pass xD @oblitum you are right though and I have no idea why I removed the %s... |
Added #2349 to fix this. |
@w0rp , please |
As requested in #1516 and #2154, this will add support for the super awesome grammar/style checker languagetool
I provide this without any tests
Let me know if you have any feedback!