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

clangcheck: Add -fno-color-diagnostics (closes #2188) #2700

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

elebow
Copy link
Member

@elebow elebow commented Aug 15, 2019

Also change to the modern --extra-arg syntax.

clang has supported -fno-color-diagnostics for at least ten years: llvm-mirror/clang@a46c71a


This works on my machine, but I don't use clang often so a second tester would be appreciated.

@@ -20,7 +20,7 @@ function! ale_linters#cpp#clangcheck#GetCommand(buffer) abort
" being generated. These are only added if no build directory can be
" detected.
return '%e -analyze %s'
\ . (empty(l:build_dir) ? ' -extra-arg -Xclang -extra-arg -analyzer-output=text' : '')
\ . (empty(l:build_dir) ? ' --extra-arg="-Xclang" --extra-arg="-analyzer-output=text" --extra-arg="-fno-color-diagnostics"': '')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon it's worth removing the double quotes here, as double quotes can sometimes break commands on Windows. Processing double quotes in a shell in Windows is the responsibility of the command instead of the shell, which causes issues where you sometimes end up passing "foo" to a Windows program instead of foo.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double quotes removed.

Also change to the modern --extra-arg syntax.
@elebow elebow force-pushed the clangcheck-no-color-diagnostics branch from f3b3ab7 to 6260256 Compare September 27, 2019 00:28
@w0rp w0rp merged commit 0d4dfb6 into dense-analysis:master Oct 28, 2019
@w0rp
Copy link
Member

w0rp commented Oct 28, 2019

Cheers! 🍻

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.

2 participants