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

WIP: Add cppcheck includes when modified buffer or no compile_commands.json #2506

Closed
wants to merge 14 commits into from

Conversation

ckoehler
Copy link

This can probably be expanded to look at -I directives from compile_commands.json to make it more robust. See comments in the code.

I don't know how to modify a buffer from a Vader Execute block, so those tests are failing right now. You may have a better idea.

Christoph Koehler added 4 commits May 15, 2019 09:18
If a file is modified, using compile_commands.json doesn't work, because
it doesn't take into account the local changes. Instead, include the
file's directory in the cppcheck call with -I. Also do this if
compile_commands.json isn't found.

This could be expanded to add other -I flags to cppcheck based on the
command in compile_commands.json, if found.
@w0rp
Copy link
Member

w0rp commented May 15, 2019

You can use :set modified to mark a buffer as modified.

Is there any danger in always setting -I with the directory name? Does it make the results less accurate? If so, your solution is a good one. Paired with the code I had for improving how compile_commands.json files are found, this should be good.

@ckoehler
Copy link
Author

I'm not sure it'd make a difference. I think cppcheck just ignores -I when --project is given.
Doing a :set modified or set modified does not seem to work in the test...

@ckoehler
Copy link
Author

Actually, after setting modified, do I need to call ALE again? I've tried

set modified
ALELint

and

set modified
call ale#Queue(0)

but neither seem to work. I'd appreciate the help, if you want that last test. I don't have time to pursue this much further.
Let me know if you figure it out, or whether I can just remove the last test. I'll then clean up the pull request.

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

The duplicate code can be written in one function here. If you can get the tests to work, let me know. If not, I or someone else might pick this up later.

ale_linters/c/cppcheck.vim Outdated Show resolved Hide resolved
@w0rp w0rp added this to In Review in Old Working List via automation May 16, 2019
@ckoehler
Copy link
Author

I escaped the paths.

Can you clarify what you think should be a separate function, and where it should live?

@w0rp
Copy link
Member

w0rp commented May 20, 2019

You can create a function in autoload/ale/handlers/cppcheck.vim that both versions of the linter can use. Make sure to update the code here to take the recent changes I've made for using build/compile_commands.json into account.

@ckoehler
Copy link
Author

Alright, refactored, and all but that one test (where I set modified) pass. I don't know enough about Vader to figure this one out, so I'll leave it up to you from here. Thanks!

@w0rp
Copy link
Member

w0rp commented May 29, 2019

@bartlibert Does this look okay to you?

@bartlibert
Copy link
Contributor

Looks fine to me. Nice improvement.

@w0rp
Copy link
Member

w0rp commented Jun 3, 2019

I fixed the test and merged this via the commandline now.

@w0rp w0rp closed this Jun 3, 2019
Old Working List automation moved this from In Review to Done Jun 3, 2019
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.

None yet

3 participants