google / vim-codefmt

Vim plugin for syntax-aware code formatting
Apache License 2.0
1.11k stars 114 forks source link

clang-format version detection is slightly broken (and definitely so for trunk builds) #188

Open malcolmr opened 2 years ago

malcolmr commented 2 years ago

At https://github.com/google/vim-codefmt/blob/293c208/autoload/codefmt/clangformat.vim#L32-34, we have this code:

    let l:version_string = matchstr(l:version_output, '\v\d+(.\d+)+')
    " If no version string was matched, cached version will be an empty list.
    let s:clang_format_version = map(split(l:version_string, '\.'), 'v:val + 0')

which is trying to find a version number (possibly "1.2.3" or "1.2" or just "1") in the output of clang-format --version:

$ clang-format --version
clang-format version 7.0.1-8+deb10u2 (tags/RELEASE_701/final)

However, for trunk builds, the output looks something like:

 clang-format --version
clang-format version mainline (4321b9f2e9842982d13234920a643e3a4657c60b)

(where "mainline" can be any string the configurer chooses).

However:

We probably should do something like:

  1. Change the regexp to use \. instead to match a literal.
  2. Consider a non-standard version to always have the feature rather than not have it (at https://github.com/google/vim-codefmt/blob/293c208/autoload/codefmt/clangformat.vim#L38.
dbarnett commented 2 years ago

For "2. Consider a non-standard version to always have the feature" — That's just to say we have no idea, so we'll err on the side of trying to use the new feature?

malcolmr commented 2 years ago

Right, if we can't tell, then assume that the feature is present.