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

Add definition of c/clangd's language as C #2791

Merged
merged 3 commits into from
Oct 17, 2019
Merged

Add definition of c/clangd's language as C #2791

merged 3 commits into from
Oct 17, 2019

Conversation

RageKnify
Copy link
Contributor

Fixes #2785

@RageKnify
Copy link
Contributor Author

The clangd test needs to be updated, I'm looking at the clang test as a guide.

@w0rp w0rp merged commit f4070f6 into dense-analysis:master Oct 17, 2019
@w0rp
Copy link
Member

w0rp commented Oct 17, 2019

Cheers! 🍻

AntoineGagne added a commit to AntoineGagne/ale that referenced this pull request Oct 31, 2019
* master: (39 commits)
  Fix the test issues with html-beautify
  Add support for html-beautify (dense-analysis#2788)
  fixers/stylelint: enhance `stylelint` fixer (dense-analysis#2745)
  Fix dense-analysis#2835 - Bump up the sign group version check for NeoVim
  Mention the disabled option for message severity
  Adding support for LSP `window/showMessage` method (dense-analysis#2652)
  Fix tsserver not returning details for items with empty source
  Allow code actions to work on callback based sources
  Add support for nimlsp (dense-analysis#2815)
  Add definition of c/clangd's language as C (dense-analysis#2791)
  Bump the ALE version
  Fix TCP server config example.
  Suboptimal fix to prevent variables from leaking out of new clangd test
  Hopefully fixed issue with Windows paths
  Added tests for clangd compile commands dectection
  Updated ale_linters/c/clangd.vim to match ale_linters/cpp/clangd.vim
  Fix dense-analysis#2800 - Ignore completion user data which is not a dictionary
  Fix dense-analysis#2821 - Fix the debride linter after merging older code
  Add the possiblity to add extra psalm options
  fix tflint handler for 0.11+ (dense-analysis#2775)
  ...
@khalsah
Copy link

khalsah commented Nov 3, 2019

@w0rp I believe this change should be reverted as it appears to have broken clangd support on c files. I expect the disappearance of C++ warnings mentioned in #2785 is due to clangd no long running, rather than it correcting the file type.

Running against the latest master of ale, and clangd 9, with only the clangd linter enabled, I observed that clangd was not reporting any errors, warnings, or completions in my project. In attempting to track down I cause I noticed this change and when I removed the -x c flags from the clangd linter definition functionality was restored.

I tested clangd versions 7, 8, and 9 to verify this was not simply a change in behavior across versions. In each case, when run from the command line with clangd -x c the command exited with errors of the form:

clangd: Unknown command line argument '-x'.  Try: 'clangd --help'
clangd: Did you mean '  -h'?
clangd: Unknown command line argument 'c'.  Try: 'clangd --help'

timlag1305 pushed a commit to timlag1305/ale that referenced this pull request Nov 5, 2019
* Add definition of c/clangd's language as C
* Update tests for clangd to be called with '-x c'
* Change to use single quotes instead of double quotes
@RageKnify
Copy link
Contributor Author

I assumed clangd had the -x option like clang, since it doesn't it seems like it's intended to be used solely for c++.

@khalsah
Copy link

khalsah commented Nov 6, 2019

@RageKnify I'm new to clangd myself, but my understanding is that clangd expects either a compile_commands.json or a compile_flags.txt file in the project to configure those details: https://clang.llvm.org/extra/clangd/Installation.html#project-setup (This also allows it to correctly handle projects with a mix of settings across files).

@w0rp
Copy link
Member

w0rp commented Nov 7, 2019

Okay, I'll revert that change now.

w0rp added a commit that referenced this pull request Nov 7, 2019
@w0rp
Copy link
Member

w0rp commented Nov 7, 2019

I have reverted that now.

AntoineGagne added a commit to AntoineGagne/ale that referenced this pull request Nov 20, 2019
* master:
  Clean up the nimpretty code
  add nimpretty fixer
  Default errorview in pwsh7 now concise
  Add tagged entry for symbols to documentation
  Switch variables to dictionary key
  Add scriptencoding to `completion.vim`
  Fix typo
  Add documentation for ale-symbols feature
  Allow the user to set their own completion values
  Switch from style to transformers (dense-analysis#2838)
  Fix a test failing in AppVeyor
  Revert "Add definition of c/clangd's language as C (dense-analysis#2791)"
  Add nimcheck end_col options
  Allow the use of StandardX for StandardJS linting and fixing. See https://github.com/standard/standardx
  Remove standardts fixer in favor of allowing standard.vim fixer to handle JavaScript or TypeScript options.
  Make sure README and docs are synced.
  Add StandardJS for TypeScript linting and fixing.
  Fix crystal-lang non file-tied message handling
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.

c/clangd.vim not defining the language as c
3 participants