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

New CLI subcommand to create clang-compatible compilation database (compile_commands.json) #14370

Merged
merged 44 commits into from
Sep 16, 2021

Conversation

baodrate
Copy link

Description

Updating @xton's PR #10264, which adds a CLI command to generate a Clang Compilation Database file, for use with language servers, linters, and other tooling. No major changes other than addressing merge conflicts.

I changed the subcommand name, as was requested in the review for the previous PR, but to generate-compilation-database instead of generate-compile-commands as @skullydazed suggested. This seems more aligned with how the file is referred to in compatible tools' documentation. But I can change this if needed.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@zvecr zvecr changed the base branch from master to develop September 10, 2021 04:35
@tzarc
Copy link
Member

tzarc commented Sep 10, 2021

Any time I try to execute it gives this error, regardless of the parameters:

☒ Could not determine keyboard!
☒ You must supply both `--keyboard` and `--keymap`, or be in a directory for a keyboard or keymap.

You'll also need to remove the debugging statements when printing out loading of modules.

@baodrate
Copy link
Author

Any time I try to execute it gives this error, regardless of the parameters

Hmm, I have those settings saved to my config so I didn't notice that. I'll figure it out

You'll also need to remove the debugging statements when printing out loading of modules

agh I didn't realize I committed those print statements. that's embarrassing 😅

@baodrate
Copy link
Author

Fixed the issue. I missed the config parameters when I renamed the subcommand, so they weren't being parsed correctly.

Also added error handling for the case where no commands were generated (e.g. you specified a non-existent keymap) and some minor improvements (instead of checking checking exit status or calling which ... ourselves, delegate it to Popen/shutil, respectively)

@baodrate
Copy link
Author

Now using cli.run() instead of subprocess.Popen(). It now waits for the command to complete before parsing the output from make but there's relatively little output so the impact to overall run time is negligible.

We will still likely need to fix the system_libs() function to give reasonable output for windows environments. I literally never use windows so I'll have to do some research to figure out what that will look like.

@AWare
Copy link

AWare commented Sep 14, 2021

Just a quick note of thanks, this PR has helped me make vscode a lot happier about qmk 👏

@drashna drashna requested a review from a team September 16, 2021 03:28
@tzarc tzarc requested a review from skullydazed September 16, 2021 04:59
@tzarc tzarc merged commit 590b405 into qmk:develop Sep 16, 2021
cadusk pushed a commit to cadusk/qmk_firmware that referenced this pull request Sep 19, 2021
* qmk/develop: (69 commits)
  Use opendrain pin with external pullup again (qmk#14474)
  Add i2c defaults for Convert to Proton C (qmk#14470)
  [Keyboard] Increase the way to add oled code for helix/rev3. (qmk#14426)
  [Bug] fix logical minimum in Programmable Button rdesc (qmk#14464)
  Adds optional hebrew layout (Unicode) (qmk#14156)
  New CLI subcommand to create clang-compatible compilation database (`compile_commands.json`) (qmk#14370)
  [Keyboard] Add the Idobao ID96 keyboard (qmk#14371)
  [Keyboard] Add FJLabs TF60 Variants and TF65 Variant (qmk#14392)
  [Keyboard] Add Absolute Designs AD65 Keyboard (qmk#14391)
  [Bug] Fix descriptor for USB Programmable Buttons (qmk#14455)
  Make ChibiOS PAL interactions less STM32 specific - Round 2 (qmk#14456)
  core: fix compilation issues with USB programmable buttons (qmk#14454)
  [Keyboard] add Matrix Me (qmk#14331)
  [Keyboard] Replaced Maker Keyboards & FJLabs Legacy Code (qmk#14393)
  [Keyboard] Add `NO_LED` positions to match key matrix. (qmk#14417)
  [Keymap] A slight improvement to my own ErgoDox keymap (qmk#14425)
  [Keymap] Trying again with Prime-e update! (qmk#14429)
  [Keyboard] Update lighting effects for xbows keyboard (qmk#14432)
  [Docs] add sync options heading, update led indicators (qmk#14441)
  [Bug] Fix IS31fl3741 driver to accept 1 or 2 addresses (qmk#14451)
  ...
ptrxyz pushed a commit to ptrxyz/qmk_firmware that referenced this pull request Apr 9, 2022
…compile_commands.json`) (qmk#14370)

* pulled source from dev branch

* missed a file from origin

* formatting

* revised argument names. relaxed matching rules to work for avr too

* add docstrings

* added docs. tightened up regex

* remove unused imports

* cleaning up command file. use existing qmk dir constant

* rename parser library file

* move lib functions into command file. there are only 2 and they aren't large

* currently debugging...

* more robustly find config

* updated docs

* remove unused imports

* reuse make executable from the main make command

* pulled source from dev branch

* missed a file from origin

* formatting

* revised argument names. relaxed matching rules to work for avr too

* add docstrings

* added docs. tightened up regex

* remove unused imports

* cleaning up command file. use existing qmk dir constant

* rename parser library file

* move lib functions into command file. there are only 2 and they aren't large

* currently debugging...

* more robustly find config

* updated docs

* remove unused imports

* reuse make executable from the main make command

* remove MAKEFLAGS from environment for better control over process management

* Update .gitignore

Co-authored-by: Michael Forster <[email protected]>

* add a usage line to docs

* doc change as suggested

Co-authored-by: Nick Brassel <[email protected]>

* rename command

* remove debug print statements

* generate-compilation-database: fix arg handling

* generate-comilation-db: improve error handling

* use cli.run() instead of Popen()

Co-authored-by: Xton <[email protected]>
Co-authored-by: Christon DeWan <[email protected]>
Co-authored-by: Michael Forster <[email protected]>
Co-authored-by: Nick Brassel <[email protected]>
@Diaoul
Copy link
Contributor

Diaoul commented Jan 1, 2024

Trying to use this I still have a few issues in vscode in keymaps after generating the database:

Unknown type name 'uint16_t' (fix available) clang(unknown_typename)

and

Expected ';' after top level declarator (fix available) clang(invalid_token_after_toplevel_declarator)

Do I need specific clangd options or anything I am missing? Compiling an flashing work fine though 🤔

Example from keyboards/splitkb/kyria/keymaps/default:
image

BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
…compile_commands.json`) (qmk#14370)

* pulled source from dev branch

* missed a file from origin

* formatting

* revised argument names. relaxed matching rules to work for avr too

* add docstrings

* added docs. tightened up regex

* remove unused imports

* cleaning up command file. use existing qmk dir constant

* rename parser library file

* move lib functions into command file. there are only 2 and they aren't large

* currently debugging...

* more robustly find config

* updated docs

* remove unused imports

* reuse make executable from the main make command

* pulled source from dev branch

* missed a file from origin

* formatting

* revised argument names. relaxed matching rules to work for avr too

* add docstrings

* added docs. tightened up regex

* remove unused imports

* cleaning up command file. use existing qmk dir constant

* rename parser library file

* move lib functions into command file. there are only 2 and they aren't large

* currently debugging...

* more robustly find config

* updated docs

* remove unused imports

* reuse make executable from the main make command

* remove MAKEFLAGS from environment for better control over process management

* Update .gitignore

Co-authored-by: Michael Forster <[email protected]>

* add a usage line to docs

* doc change as suggested

Co-authored-by: Nick Brassel <[email protected]>

* rename command

* remove debug print statements

* generate-compilation-database: fix arg handling

* generate-comilation-db: improve error handling

* use cli.run() instead of Popen()

Co-authored-by: Xton <[email protected]>
Co-authored-by: Christon DeWan <[email protected]>
Co-authored-by: Michael Forster <[email protected]>
Co-authored-by: Nick Brassel <[email protected]>
@user78552
Copy link

@Diaoul Hi, did you manage to solve this problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

YouCompleteMe support for auto completion
7 participants