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

Go to definition #124

Closed
rchl opened this issue Oct 30, 2016 · 20 comments
Closed

Go to definition #124

rchl opened this issue Oct 30, 2016 · 20 comments

Comments

@rchl
Copy link
Contributor

rchl commented Oct 30, 2016

It would be great to be able to jump to object's definition or declaration using a shortcut. It's possible to do as SublimeClang plugin is/was supporting that and that is pretty much the only thing from that plugin that I'm missing in EasyClangComplete currently.

That also could be made to work with #include directives to quickly jump to a header file (as proved to be possible by that other plugin).

(Often sublime's ctrl+shift+r does a good job of finding declarations/definitions but it's not ideal)

@niosus
Copy link
Owner

niosus commented Oct 30, 2016

Yes, I have been thinking about it.

The problem for me here is that this requires fixing all the design issues and I would need at least a week of full time work on this plugin for this. Unfortunately, I do not have the time. Maybe when we fix all the other issues, we could really work on this.

Also, I am quite happy with how ctrl+shift+r and F12 work, so this is for now quite low priority for me. But i am open to pull requests on this.

@rasgo-cc
Copy link

rasgo-cc commented Nov 22, 2016

+1 for this! I understand the priorities though. In any case, any pointers on how to get started on implementing this feature for anyone willing to try? (maybe me if I'm up for the task)

@niosus
Copy link
Owner

niosus commented Nov 22, 2016

I really didn't look into this, but it seems that one could start along the lines of this blog post: http://eli.thegreenplace.net/2011/07/03/parsing-c-in-python-with-clang/

Until I will have found the time to dive into this I would be willing to accept a pull request on this if someone implements this :)

@simia
Copy link
Contributor

simia commented Dec 2, 2016

This seems to be quite easy to implement. I'll have a try with this one.
Btw. what's your opinion on implementing functionality like "Implement function" where plugin inserts definition into source file based on header contents?

@niosus
Copy link
Owner

niosus commented Dec 2, 2016

@simia great to hear that 👍

As for the functionality - it is a complicated question. I think I like the idea, but it has to be working properly most of the times so that it does not annoy users. So, bottom line, if it is well implemented, why not? :)

But please, separate these things in multiple PRs should you consider to actually implement this.

@simia
Copy link
Contributor

simia commented Dec 2, 2016

Ok. After a brief reading I have some initial thoughts.
Implementing goto definition/declaration requires knowledge from multiple translation units.
To achieve this we would need to preparse whole project and gather all TUs. This would be quite a huge change in terms of how plugin works and I don't if @niosus want's to go that way.
On the upside:

  • This would enable implementation of fairly large amount of features - goto definition/declaration, show usages, peek definition, implement methods/virtual methods, fix includes(?). Those are just few that come to my head right away.
  • This could be separated from current functionality and be by default disabled.

There are lots of possible obstacles on the way but I think this would be cool to have such a rich functionality within one plugin.
What are your thoughts @niosus ?

@niosus
Copy link
Owner

niosus commented Dec 2, 2016

Keeping all things compiled is a no go. It will grow out of hands really quickly. I think we should separate the functionality even more and address this really granularly. So I would not implement it project wide, but rather view dependent.

  • goto definition/declaration - we actually don't need more information than we already have. The declaration is going to be in the ast tree of the translation unit already if I understand it correctly. Definition is a bit trickier, but we can guess it is in a corresponding source file.
  • implement methods - again, we don't need more information then we have. Having a header opened we have all the information we need. The user then can specify which file to add the definitions to. What seems to be difficult to me is to keep track of which ones have already been defined, etc.

And yes, it should be separated from current functionality. I also see a lot of obstacles on the way to actually have this working reliably. If there is something that is working we could roll it out as a beta experience if it does not interfere with the completion functionalities of the plugin.

@maddanio
Copy link

Yes, going to declaration would be a huge win! especially when jumping to headers of depended upon libraries. Often there is documentation in those headers, or related functions (like the constructor set of a class), and this would make my workflow so much quicker. I tried looking at the ecc source, but got lost :). If I ever grok it properly I am open to helping :).

@simia
Copy link
Contributor

simia commented Jul 13, 2017

There is 'go to declaration' functionality on the popup that is displayed when hovering over function/variable etc.

@dibalavs
Copy link
Contributor

Looks like I did something similar. Please see pullrequest #325

@niosus
Copy link
Owner

niosus commented Oct 25, 2017

Ok, so one of the sleepless nights I've been thinking about it. @simia is totally right that we need the knowledge from multiple translation units to navigate to definition. Even more importantly, we need the knowledge of from which files these translation units need to be generated.

IDEs indeed index the whole project and as a result of this, rely heavily on their own project definition. I would really like to keep the way this plugin allows any build system and is not too heavy on plugin-specific configuration files. But even if we knew which files to parse, we would still need a way to reuse parts of this huge translation unit. Alternative is only to keep a TU for each file and this simply does not scale.

I do not really see any way to provide this functionality in any meaningful way. I encourage somebody else to write a plugin that would mimic an IDE within sublime text and would constantly parse the whole project tree. I believe it is realistic to do if some constraints on the build system or on the project structure are enforced.

I see, however, no way of implementing this within the framework of this plugin for now. All the implementations that I can think of have one or another flaw that I see no way of overcoming.

As a result, I guess I will keep this issue open here for other people to find, but I do not intend to work on it mainly because I do not see how. I will be happy if anybody actually proves me wrong.

@niosus
Copy link
Owner

niosus commented Oct 25, 2017

@maddanio and @simia I am removing some of your off-topic comments from this thread to streamline this whole conversation so that its easier to read. Sorry for that.

Repository owner deleted a comment from maddanio Oct 25, 2017
Repository owner deleted a comment from simia Oct 25, 2017
Repository owner deleted a comment from simia Oct 25, 2017
Repository owner deleted a comment from maddanio Oct 25, 2017
Repository owner deleted a comment from simia Oct 25, 2017
Repository owner deleted a comment from maddanio Oct 25, 2017
@niosus niosus changed the title Go to definition/declaration Go to definition Oct 25, 2017
@maddanio
Copy link

as a small note: I would like "whole project indexing" at least to be switchable. this kind of indexing tends to lead to massive cpu hunger of modern IDEs, and more often than not massive lags and other problems when trying to use it while changing lots of files.

@maddanio
Copy link

This problem is one of the reasons I like to use sublime instead, so I would not like it to resurface in my new home :).

@niosus
Copy link
Owner

niosus commented Oct 25, 2017

Probably I have written too much text above 😆

To have a shorter statement and to avoid confusion: there is no plan to implement go_to_definition as it would require full project indexing which does not fit with the core idea and architecture of the plugin and is overall just infeasible at this point.

@niosus niosus moved this from TODO to Ignore in EasyClangComplete Oct 25, 2017
@dibalavs
Copy link
Contributor

It is possible to use some "tricks" to get symbol definition.
we already know header file, where function is declared, So we can switch to source file (like Alt+O hotkey) and index only this source, where function is defined.
It will not work in all cases, but it is better than nothing.

@niosus
Copy link
Owner

niosus commented Oct 26, 2017

Yeah, the issue with "tricks" is that everybody will want their own. Arguably, F12 works decently well in simple cases and we will not be able to cover complicated ones with this solution.

@maddanio
Copy link

maddanio commented Nov 3, 2017

Well, then "opt-in" to this feature would be an option? :)
Is there a kind of index cache right now? Or are they recomputed all the time?

@niosus
Copy link
Owner

niosus commented Dec 7, 2017

@maddanio there is no index right now as I explicitly wanted to avoid that. Every translation unit is completely independent from another. Whenever you open a file a new translation unit is generated. The existing ones for open files are stored in cache for some time and then discarded. They are also removed if the view is closed.

@niosus
Copy link
Owner

niosus commented Dec 7, 2017

I will close this issue as I see no way of fixing it. Therefore the wontfix label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

6 participants