Fix chktex highlighting wrong column when using tabs instead of spaces

[Jorengarenar]

^{Repetition of #4661, taking into account #4712}

Fixes #723

chktex implemented bug #56486: Allow setting options on the command line
Thanks to that we can tell it to treat tab character as of one space width, i.e. one char.
That means, after we translate the output back to Vim columns, we get correct numbers.

[hsanson]

The -S flag is not available in chktex 1.7.6 that is default in some distributions (e.g. Ubuntu) so this change breaks ALE for many users. We have to make the -S flag optional based on version of chktex.

Or we can leave this out altogether since the -S TabSize=8 seems to be the default and can be changed via g:ale_tex_chktex_options or chktexrc configuration.

[Jorengarenar]

@hsanson I had added a check if the option is available

[hsanson]

After installing latest chktex 1.7.8 and doing several test I was able to understand what this PR tries to do. I made some modifications (see diff below) that you can add to this PR to improve it:

1. Use async RunWithVersionCheck() instead of calling blocking system() function.
2. Use &tabstop option instead of hardcoded 1 so it works with different vim configurations.
3. Honor .chktexrc file over hardocded -S TabSize if it exists.
4. Add tests and update current ones to test command arguments based on chktex version.

Save the following diff in a file and use git-apply to apply it:

diff --git a/ale_linters/tex/chktex.vim b/ale_linters/tex/chktex.vim
index dd064127..ca336326 100644
--- a/ale_linters/tex/chktex.vim
+++ b/ale_linters/tex/chktex.vim
@@ -4,22 +4,28 @@
 call ale#Set('tex_chktex_executable', 'chktex')
 call ale#Set('tex_chktex_options', '-I')
 
-function! ale_linters#tex#chktex#GetCommand(buffer) abort
+function! ale_linters#tex#chktex#GetExecutable(buffer) abort
+    return ale#Var(a:buffer, 'tex_chktex_executable')
+endfunction
+
+function! ale_linters#tex#chktex#GetCommand(buffer, version) abort
     let l:options = ''
 
     " Avoid bug when used without -p (last warning has gibberish for a filename)
     let l:options .= ' -v0 -p stdin -q'
 
-    " Avoid bug of reporting wrong column when using tabs (issue #723)
-    if system('chktex -S') !~? 'invalid option'
-        let l:options .= ' -S TabSize=1'
-    endif
-
     " Check for optional .chktexrc
     let l:chktex_config = ale#path#FindNearestFile(a:buffer, '.chktexrc')
 
     if !empty(l:chktex_config)
         let l:options .= ' -l ' . ale#Escape(l:chktex_config)
+    else
+        " If no configuration file is found try to set TabSize value based
+        " on buffer tabstop (#723). Note -S option is available on chktex
+        " 1.7.7+.
+        if ale#semver#GTE(a:version, [1, 7, 7])
+            let l:options .= ' -S TabSize=' . &tabstop
+        endif
     endif
 
     let l:options .= ' ' . ale#Var(a:buffer, 'tex_chktex_options')
@@ -49,7 +55,12 @@ endfunction
 
 call ale#linter#Define('tex', {
 \   'name': 'chktex',
-\   'executable': {b -> ale#Var(b, 'tex_chktex_executable')},
-\   'command': function('ale_linters#tex#chktex#GetCommand'),
+\   'executable': function('ale_linters#tex#chktex#GetExecutable'),
+\   'command': {buffer -> ale#semver#RunWithVersionCheck(
+\       buffer,
+\       ale_linters#tex#chktex#GetExecutable(buffer),
+\       '%e --version',
+\       function('ale_linters#tex#chktex#GetCommand'),
+\   )},
 \   'callback': 'ale_linters#tex#chktex#Handle'
 \})
diff --git a/test/linter/test_tex_chktex.vader b/test/linter/test_tex_chktex.vader
index 94ce0d49..1e037004 100644
--- a/test/linter/test_tex_chktex.vader
+++ b/test/linter/test_tex_chktex.vader
@@ -1,21 +1,44 @@
 Before:
   call ale#assert#SetUpLinterTest('tex', 'chktex')
 
+  GivenCommandOutput ['ChkTeX v1.7.6 - Copyright 1995-96 Jens T. Berger Thielemann']
+
 After:
   call ale#assert#TearDownLinterTest()
 
 Execute(The default command should be correct):
-  AssertLinter 'chktex',
+  AssertLinter 'chktex', [
+  \ ale#Escape('chktex') . ' --version',
   \ ale#Escape('chktex')
-  \   . ' -v0 -p stdin -q -S TabSize=1'
-  \   . ' -I'
+  \   . ' -v0 -p stdin -q'
+  \   . ' -I',
+  \]
+
+  " The version check should be cached.
+  GivenCommandOutput []
+  AssertLinter 'chktex', [
+  \ ale#Escape('chktex')
+  \   . ' -v0 -p stdin -q'
+  \   . ' -I',
+  \]
+
+  " Try newer version
+  call ale#semver#ResetVersionCache()
+  GivenCommandOutput ['ChkTeX v1.7.8 - Copyright 1995-96 Jens T. Berger Thielemann']
+  AssertLinter 'chktex', [
+  \ ale#Escape('chktex') . ' --version',
+  \ ale#Escape('chktex')
+  \   . ' -v0 -p stdin -q'
+  \   . ' -S TabSize=' . &tabstop
+  \   . ' -I',
+  \]
 
 Execute(The executable should be configurable):
   let g:ale_tex_chktex_executable = 'bin/foo'
 
   AssertLinter 'bin/foo',
   \ ale#Escape('bin/foo')
-  \   . ' -v0 -p stdin -q -S TabSize=1'
+  \   . ' -v0 -p stdin -q'
   \   . ' -I'
 
 Execute(The options should be configurable):
@@ -23,5 +46,5 @@ Execute(The options should be configurable):
 
   AssertLinter 'chktex',
   \ ale#Escape('chktex')
-  \   . ' -v0 -p stdin -q -S TabSize=1'
+  \   . ' -v0 -p stdin -q'
   \   . ' --something'

[Jorengarenar]

@hsanson
The fix of issue #723 is precisely to set TabSize to 1; no other value makes sense here, because tab is one character and that's how we want chktex to treat it, so we get correct column number in Vim.

[hsanson]

@Jorengarenar after further testing it seems you are correct and TabSize must be fixed to 1. You can modify the diff to replace &tabstop' with 1` in the linter and test files.

[hsanson]

Thanks for the work and patience.