-
Notifications
You must be signed in to change notification settings - Fork 99
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
Eldoc support #166
base: master
Are you sure you want to change the base?
Eldoc support #166
Conversation
a99249d
to
22fd810
Compare
The bulk of this commit is slightly dull code for printing information about libclang cursors in sexp form.
22fd810
to
db6026b
Compare
Hello ikirill, I will have to test this evening at home but the common thread between all the pull requests you have provided and where I'm reluctant is the complexity of the resulting code. See that I have 26 pages to scroll when looking at https://github.com/Sarcasm/irony-mode/pull/166/files scares the shit out of me. Thinks like that: https://github.com/Sarcasm/irony-mode/pull/166/files#diff-4009c1159289a1fabe776a2396944394R10 should be avoided if possible. It is just a maintenance burden to keep this kind of mapping in sync with the elisp part, and until now it hasn't been needed. Maybe one day it will but right now I'd like to think it is not. A "long time" ago for completion I was trying to provide exactly what libclang was providing, mapping kinds to elisp symbols and stuff like that. By operating some transformation on the irony-server side, I don't think I have really limited what the Elisp clients can do but I have greatly simplified the implementation (and sometime performances, though there is still a lot of work to do here).
I would prefer that you stick to you use case for now. If what you want to do is provide the current expression/scope boundaries to perform semantic kill of expression, I would like to see that, if what you want is to provide irony-eldoc context information, I would like to see that as well. Also if you could provide a sample code and what is the output for the expression at point that would be great. I know I can get this by testing the pull request but maybe there are things to discuss before implementing it. I feel very bad refusing all your hard work, I hope you continue to work with me on improving irony-mode. |
Most of it is C++-style boilerplate, and a huge chunk is just parsing the output of libclang functions. The code that decides what gets printed is in Irony.cpp, everything else is just print statements.
No, this is strictly for eldoc. I really don't want to have to modify the server whenever I want to display something differently in eldoc, and if libclang provides some information that is helpful to me in lisp, I want to be able to use it. Also, I want to point out two things. First, there are many different things things that can be seen in C++ code, and providing libclang's output to emacs-lisp code in the form of expressive association lists is a clean way to get emacs-lisp code to understand what is actually there in the file. You can return an opaque string and just tell emacs-lisp to display that, but that's just crufty. Libclang doesn't really make any strong promises about what the AST will look like. Second, C++ doesn't really change all that much, libclang's interface should be reasonably stable. I don't know what version you're targeting, though. If some enum value is unknown, it will just return the unknown symbol, with no harm done. I generated the switch statements automatically from Index.h. Third, you can't just rely on completion returning you the right results, it's not that clever. For example, for a dependent type in a call expression, there will be no completion for the call expression itself, and there will be several completion strings for the function being called. A reasonably clean way of handling this is to tell emacs-lisp code that there is some complexity in the return value, and let it handle the complexity however it wants. For example:
This is a fairly uniform structure: everything is an association list.
ikirill@db6026b#diff-90f3661683daeb04226a61e026a6f7afR477 I use this file for testing. #include <vector>
#include <string>
#include <algorithm>
// #include <boost/geometry.hpp>
// #include <boost/geometry/geometries/point_xy.hpp>
// #include <boost/geometry/geometries/polygon.hpp>
// using namespace boost::geometry;
// using namespace std;
using std::string;
using std::vector;
/** docstring for f */
void f(string x);
/** docstring for f(int, int) */
void f(int x, int y);
/** docstring for f(double, double) */
void f(double x, double y);
/** docstring for f(string, string) */
void f(string x, string y);
/** docstring for g2(string&). */
void g2(string& x) { }
/** docstring for h<T> */
template <typename T>
T h(const T& x);
/** docstring for X. */
struct X { };
/** docstring for h(X). */
X h(const X& x);
// Overloading cursors
/** docstring for h2(T) */
template <typename T>
void h2(T t) {
h(t) + h(t) + h(t) * h(t);
}
struct Y { };
/** docstring for h(Y). */
Y h(const Y& y);
/** SomeStruct docstring */
template <typename Type1, typename Type2 = float>
struct SomeStruct { Type1 elt1; Type2 elt2; };
/** docstring for g
\param x g-docstring/param-x
*/
void g(/** g-arg-x-1 */ string /** g-arg-x-2 */ x /** g-arg-x-3 */ ) {
/** docstring y */
vector<float> y;
y = vector<float>(10, 0.1);
sort(y.begin(), y.end());
sort(y.begin(), y.end());
f(x);
f(x, /* comment */ x);
g(x); // x is bound to a temporary
g2(x); // x is not bound to a temporary
f(x,
x);
h(x);
/** SomeStruct ss1 docstring */
SomeStruct<double> ss1;
/** SomeStruct ss2 docstring */
SomeStruct<double, double> ss2;
// model::d2::point_xy<int> p1(1, 1), p2(2, 2);
// std::cout << "Distance p1-p2 is: " << distance(p1, p2) << std::endl;
int z = 1, w = 2, p = z + w + w + NULL;
}
struct X;
// Local Variables:
// eval: (when (fboundp (quote flycheck-mode)) (flycheck-mode -1))
// End: It's a bit tedious to write library bindings in a non-automated way, but definitely not hard. The build fails right now, because it's using some old version of libclang, I think. The fact that you can select some arbitrary expression/region (not just a symbol) and find out its type (technically, the type of the innermost enclosing expression) is quite nice, I think. This PR being 1k lines long and tied to a recent version of libclang (it needs getTypeSpelling, I think most other things can be commented/ifdef'd out easily) is unfortunate. You can even comment out getTypeSpelling and just print nil instead of type spellings. |
But there is no elisp part: either elisp gets a symbol it understands, and does something useful with it, or it gets a symbol it doesn't understand and just ignores it. |
I do not agree with the statement in bold. I used to return some association list from, see older versions of irony-mode (https://github.com/Sarcasm/irony-mode/blob/v0.0.1-alpha/server/plugins/CodeCompletion.cpp#L399, irony-mode/server/plugins/CodeCompletion.cpp Line 135 in 27b97c0
If you want to have access to the libclang AST in Emacs, irony-mode isn't the right tool (in the foreseeable future). |
This patch does not expose the AST to emacs code, it returns enough information to display type at point/region. Like I said, I don't want to have to modify irony-server every time something should be displayed differently, and I would like emacs code to understand, roughly, what is being displayed and why.
That is, in fact, a better design. It means that other people can reuse the output of your API in ways that you have not foreseen exactly. |
When I say I'm against the exposing the AST I wanted to say that I'm against exposing the libclang internals.
No it is too difficult to provide something stable based on this. At one time I returned things like Clang "typed text " as a cons I think we have reached, the end of the debates. I'm personally convinced that exposing libclang is a bad idea, and that is not the road I want to take for irony. If we can't agree on this I suggest we continue on separate roads, you fork irony-mode, make your another an elisp binding to libclang, whatever but I will not accept code that do not offer a precise use-case in irony-server for now. |
Following #162, this PR is smaller, and only returns information about the type of a given expression.
irony-exprtype
is an emacs-lisp function returns an association list of properties for the expression at point (or in region, if a region is active). This can be transformed to whatever is suitable for display.