-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 cmake-ide to lang/c-c++ layer #9432
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the issues: https://circleci.com/gh/syl20bnr/spacemacs/16
Your README has issues. |
The fix is literally to just change |
@robbyoconnor , thanks a lot! Now it's all green 👍 |
@robbyoconnor , do we need to perform some extra steps to merge this PR? |
@robbyoconnor , I see. Thank you for your assistance. |
Also worth noting: You _MUST_ do this against develop.
|
layers/+lang/c-c++/packages.el
Outdated
(spacemacs/declare-prefix-for-mode mode "mg" "goto") | ||
(spacemacs/declare-prefix-for-mode mode "mp" "project/build system") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the prefix be "project", not "project/build system"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the above
layers/+lang/c-c++/README.org
Outdated
@@ -112,6 +115,8 @@ doesn't complain about missing header files. | |||
| ~SPC m g A~ | open matching file in another window (e.g. switch between .cpp and .h) | | |||
| ~SPC m D~ | disaster: disassemble c/c++ code | | |||
| ~SPC m r~ | srefactor: refactor thing at point. | | |||
| ~SPC m p~ | Project / Build system management | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to add the key bindings for cmake-ide
functions. Also SPC m p
is a prefix, not a function, so it doesn't need a key binding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're not actually documenting any of the keybindings. This is rather lazy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments. Documentation needs work.
"pc" 'cmake-ide-run-cmake | ||
"pC" 'cmake-ide-maybe-run-cmake | ||
"pd" 'cmake-ide-delete-file)))) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These need to be documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
layers/+lang/c-c++/packages.el
Outdated
(spacemacs/declare-prefix-for-mode mode "mg" "goto") | ||
(spacemacs/declare-prefix-for-mode mode "mp" "project/build system") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the above
layers/+lang/c-c++/README.org
Outdated
@@ -112,6 +115,8 @@ doesn't complain about missing header files. | |||
| ~SPC m g A~ | open matching file in another window (e.g. switch between .cpp and .h) | | |||
| ~SPC m D~ | disaster: disassemble c/c++ code | | |||
| ~SPC m r~ | srefactor: refactor thing at point. | | |||
| ~SPC m p~ | Project / Build system management | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're not actually documenting any of the keybindings. This is rather lazy.
@beta1440 , @robbyoconnor , thanks for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 ✔️ ✅
"pc" 'cmake-ide-run-cmake | ||
"pC" 'cmake-ide-maybe-run-cmake | ||
"pd" 'cmake-ide-delete-file)))) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅
Note to collaborators and @syl20bnr : I didn't run this -- I simply checked if it looked okay and had supporting documentation. |
@syl20bnr, plz review this PR. I hope it's ready to merge. Note: I'm using it in my every day work ( + rtags) |
Thank you ! 👍 |
The
Is there any way to speed this up by default, or disable it? It's taking literally sometimes minutes to open a header file. The |
This seems to affect more than just C and C++ files. I went to open the latest C/C++ layer docs, and waited for all this work to finish:
|
seems like it worth to add variable to be able to enable/disable cmake-ide |
Thanks @myrgy, that helped! |
@andschwa how did you fixed it? I am having the same problem, spacemacs is unusably slow. Also, if this is broken, why is it enabled by default? |
@gnzlbg , I think it was
I'm unable to reproduce that issue - for my projects it works pretty fast. |
Are they header-only projects? How big are they? |
I've tried this and now cmake-ide goes from 10-20s to 4-5s to open header files, still pretty slow. |
I'm sorry, but I think this change needs to be reverted. The Spacemacs is supposed to "just work", and turning on For reference, I can't get
I've gone back and tested the current released version of Spacemacs with this project, and opening all my source and headers is instantaneous, as it should be. But it's not the case int he develop branch. Edit: believe me, if we could get |
Maybe we can just add a way to easily toggle it on-off, or to easily configure whether you can turn it on/off by default. I like using |
rebased to develop version of #7187
preparing stage for RTags(pull request #2834). (issue #2327)