atilaneves / cmake-ide

Use Emacs as a C/C++ IDE
BSD 3-Clause "New" or "Revised" License
714 stars 92 forks source link

Add support for irony stack #16

Closed agauniyal closed 8 years ago

agauniyal commented 8 years ago

A lot of developers have migrated from autocomplete to company mode as well as clang completions to irony mode. Personally I use :

Irony mode works with libclang, sends to company irony which also works in completing c headers. Then Irony mode works with flycheck for diagnostics. Eldoc is different thing.

atilaneves commented 8 years ago

The problem here is I don't have irony installed at all and it's going to be hard for me to include this. I suggest making a PR, it'll probably happen a lot faster that way.

agauniyal commented 8 years ago

I'm sorry I'm quite new to emacs and lisp stuff otherwise I would've made a PR. However here's something that might help in future : link.

Also could you tell difference b/w autocomplete and company mode? or how do these compare. I only transitioned from vim to emacs after your conf. And I found great community support for irony stack, hence I chose it.

atilaneves commented 8 years ago

I tried looking into irony but right now this issue is blocked by an irony issue I filed.

cviebig commented 8 years ago

+1 :)

sezaru commented 8 years ago

I've made a hack to fix this absolute path issue, it's probably not the best workaround since I don't know nothing about elisp, but so far it is working great (actually using company-irony is way, way better than company-clang, the results come instantly, no delay at all).

The hack is simply using function advice to check if the path given by cmake-ide is an absolute path, if so, I reset one variable that will force the path to continue absolute:

(defadvice irony-cdb--locate-dominating-file-with-dirs
    (before irony-cdb--locate-dominating-file-with-dirs-before
        (file name subdirectories) activate)
  "Check if some subdir is absolute, if so, set file to root."
  (cl-loop for subdir in subdirectories
       do (if (file-name-absolute-p subdir)
          (setq file "/."))))

To make cmake-ide set the currect path to irony-mode, I made a very similar approach, but you can change directly the cmake-ide code if you want:

(defadvice cmake-ide-set-compiler-flags
    (before cmake-ide-set-compiler-flags-before
        (buffer flags includes sys-includes) activate)
  "Add support for irony-mode."
  (when (buffer-live-p buffer)
    (with-current-buffer buffer
      (when (featurep 'irony)
    (setq irony-cdb-search-directory-list '())
    (add-to-list 'irony-cdb-search-directory-list (cmake-ide--get-dir))
    (irony-cdb-autosetup-compile-options)))))

Again, this is simply a hack, but until there is a official fix, it will work fine.

atilaneves commented 8 years ago

Thanks for looking into this @sezaru. If it works, would you mind creating a pull request? I can't easily test this myself.

sezaru commented 8 years ago

Sure, but let me ask first, is this really a valid pull request? I mean, the part that I add support to irony-mode to cmake-ide is ok I guess, but the hack to make irony-mode work is simply a very ugly hack in my opinion, and shouldn't be part of cmake-ide.

About testing it, you can simply put it in your init.el and it should simply work, you can see in my config repository https://github.com/sezaru/emacs-config how I configured irony-mode

sezaru commented 8 years ago

Just for completness, I look at Malinka project to see how they have solve this issue with irony-mode, the way they did was to simply copy the cmake generated json file to the root of the cmake project, this way irony-mode can see it.

I prefer my approuch compared to theirs, since it will not polute the project with a file that could be commited to a repository by mistake

agauniyal commented 8 years ago

Sorry about the silence, thanks for the ping. In this case, where the compilation database cannot be found by looking at a relative path you can call irony-cdb-json-add-compile-commands-path to add a compilation database for the given project. Putting an absolute path in irony-cdb-search-directory-list make little sense I believe unless you have one compilation database for all of your projects but if that the case, I don't think putting this path in irony-cdb-search-directory-list is the best way to express this.

From the dev of irony stack.

sezaru commented 8 years ago

I do agree with the irony dev, hence I call it a hack :)

A better solution in my view would be to add support (if cmade-ide already have it I'm unaware of) to set the cmake build dir to inside the project directory, let's say root_of_project/build for example, this way irony-mode would work out of the box, and would bring other benefits like unifying the build of the project (for now I build my project manually in the build folder for easy access to the binaries) and since it would be unified, cmake-ide could even bring new helpers like ease switch to build in debug mode, etc.

As far as problems, what I can think for now would be if the user heavily changed a CMakeLists.txt, there was cases where this broke cmake and the solution would be to recreate build and rerun cmake in it. A probably solution to this could be that cmake-ide can store md5sum of the CMakeLists.txt of the project, and whenever it would run cmake (like with the cmake-ide-run-cmake), it would check these files, and, if there was some change, it would reset the directory and rerun cmake in it.

Or, since this is really rare, a simple function that would reset the folder to be called manually would also solve it.

atilaneves commented 8 years ago

You can set cmake-ide-dir to a dir inside the project, there's no problem with that. My problem is I usually don't, and it doesn't make sense to me to limit options.