hotpxl / company-irony-c-headers

Company mode backend for C/C++ header files with Irony
GNU General Public License v3.0
30 stars 3 forks source link

Completion not working if without .clang_complete file in working directory. #5

Closed KazuSoap closed 9 years ago

KazuSoap commented 9 years ago

Hello, I noticed that completion does not work if without .clang_complete file in working directory. So I looked at company-irony-c-headers.el source and found some useless code in function of company-irony-c-headers-reload-compiler-output.

< in 89 >

  (when (and company-irony-c-headers--compiler-executable
             (company-irony-c-headers--working-dir))

I think conditional branching by (company-irony-c-headers--working-dir) cause the problem.

< in 94 >

     (let ((uco (company-irony-c-headers--user-compiler-options))
           (dco (company-irony-c-headers--default-compiler-options))
           (default-directory (company-irony-c-headers--working-dir)))

The variable of default-directory isn't use in company-irony-c-headers-reload-compiler-output.

So I removed these useless code then completion working perfectly without .clang_complete file in working directory. Could you apply this modification if there is no problem?

Sorry for my bad English. Thank you.

hotpxl commented 9 years ago

Actually the default-directory is used by call-process (as a result of dynamic scoping). I added this line in a patch. If I leave that out, and if default-directory is nil, as in some buffers not associated with files, it will just break.

I discussed this problem with the author of Irony that it won't work without a .clang_complete. It's actually really hard to determine the file type and associated info (C++ std and options). And it would not be so user-friendly to assume. So I'd rather leave that part out.

KazuSoap commented 9 years ago

Hmm... I understood the importance of default-directory. But without .clang_complete company-irony load default include path (you can dump clang++ -E -x c++ - -v < /dev/null ) and current directory, and with .clang_complete load additional path too. So I think company-irony-c-headers can also do like company-irony. How about the below code?

@@ -85,13 +85,13 @@
 (defun company-irony-c-headers-reload-compiler-output ()
   "Call compiler to get search paths."
   (interactive)
-  (when (and company-irony-c-headers--compiler-executable
-             (company-irony-c-headers--working-dir))
+  (when company-irony-c-headers--compiler-executable
     (setq
      company-irony-c-headers--compiler-output
      (let ((uco (company-irony-c-headers--user-compiler-options))
            (dco (company-irony-c-headers--default-compiler-options))
-           (default-directory (company-irony-c-headers--working-dir)))
+           (default-directory (or (company-irony-c-headers--working-dir)
+                                  default-directory)))
        (with-temp-buffer
              (apply 'call-process
                     company-irony-c-headers--compiler-executable nil t nil
hotpxl commented 9 years ago

I'm not quite familiar with Irony's behavior when no .clang_complete is present. Could you point me to the part that deals with it? If Irony is doing that, I suppose we could do that too.

KazuSoap commented 9 years ago

Sorry for my late reply because my work was busy, and thank you for your modification. Although the description may not be necessary, I try to explain irony's behavior.

Feature Highlights of irony emacs package is sending command to irony-server which uses libclang and getting the result of it. So in such cases what kind of clang compiler options irony-server receives is important. I checked the difference of command between existence/absence of .clang_complete. If it exists, -working-directory <the path of its existence> and -I<additional include path written in it> is added to the command without it. default-directory is related with -working-directory option. Try to clang --help command on terminal then you can find the description below

 -working-directory <value>  Resolve file paths relative to the specified directory

This mean if relative paths exists in the clang compiler options, resolve file paths relative to the specified directory. Thus I was sure this change(before my comment) was going to work out.

Sorry for my bad English. Thank you.

hotpxl commented 9 years ago

Thanks for the explanation. I pushed a commit and please check if it's working as expected. I tested on my own machine and it works, but the completion is not very comprehensive since no flags is provided. But anyways it just works.

The problem, however, is when I wanted to complete some STL methods. I can autocomplete #include <vector>, so I think Irony is able to find the vector library at all. But when I want to autocomplete method invocation on any vector, it doesn't give me the options. I suppose it's some behavior with Irony. I talked with the author of Irony on this. But we haven't reached any conclusion yet. After all, since no flags is provided, it's all undefined behavior.

KazuSoap commented 9 years ago

At least in my environment (Ubuntu 14.04 LTS + emacs 24.5), such problem doesn't occur. But in my another environment (Windows7 + msys2 + NTemacs 24.5), the problem which irony cannot load default include path (you can dump clang++ -E -x c++ - -v < /dev/null) occurs and this problem has been reported https://github.com/Sarcasm/irony-mode/issues/132. So in such environment, the problem you reported may occur. I cannot reported OS X environment because I don't have apple products.

hotpxl commented 9 years ago

Thanks for the feedback. I use Linux and Mac. So I don't really bother to support Windows. If it works, then great. If it doesn't, I don't know how to test it as well :)