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

feat: add compile_commands.json output to make debug build #5781

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

virchau13
Copy link
Contributor

Describe your PR, what does it fix/add?

This makes all the include resolution and diagnostic information work properly (with the clangd language server on NixOS, at least.) Probably makes this line unnecessary:

#pragma diag_suppress 1696

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

  • As far as I can tell, the xwayland dependency provides literally nothing.
/nix/store/i3yspjhpqz8n25phd91p5cyrh2jl1xvh-xwayland-23.2.6
├── bin
│  └── Xwayland
├── lib
│  ├── pkgconfig
│  │  └── xwayland.pc
│  └── xorg
│     └── protocol.txt
└── share
   ├── applications
   │  └── org.freedesktop.Xwayland.desktop
   └── man
      └── man1
         ├── Xserver.1.gz
         └── Xwayland.1.gz

I'm not sure why it's even in CMakeLists.txt in the first place.

Is it ready for merging, or does it need work?

Ready for merge.

@vaxerski vaxerski requested a review from fufexan April 28, 2024 12:42
@fufexan
Copy link
Member

fufexan commented Apr 28, 2024

Probably makes this line unnecessary:

If it's the case, can you also remove it in this PR?

@virchau13
Copy link
Contributor Author

virchau13 commented Apr 28, 2024

I don't have VSCode, so someone else should test if this actually fixes it in VSCode (I tested it in Neovim).

@BvngeeCord
Copy link

Can confirm, this fixes all include errors for me in Neovim with Clangd. Note: by default, clangd shouldn't need compile_commands.json to be symlinked to the project root, as looks upwards naturally starting from where the source file is. Though it might still be good to be safe.

Alternatively, set(CMAKE_EXPORT_COMPILE_COMMANDS TRUE) can be set inside of CMakeLists.txt instead of the env var if that is preffered (which is what I've been doing).

@vaxerski vaxerski force-pushed the main branch 2 times, most recently from fb471b8 to 1237732 Compare May 3, 2024 21:40
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