Better information when GoToDefinition fails

[abingham]

In C++, GoToDefinition will fail if the definition is in a different translation unit. We might tell that to the user when it fails.

[abingham]

We might be able to cheat. If gtd fails for c++, a goto declaration may succeed. If so, we can try to guess the cpp file from the header and redo the request. This may be too slow.

[noxdafox]

I think such logic should be handled on the backend side (ycmd).

[abingham]

In principle I agree, though I don't know if we'll be getting that feature any time soon. If it's simple to implement on the client end, it might be a quick win. But again, I think it might be too slow if it involved initiating new compilations and such.

[unhammer]

Would it be possible to run a hook on failure? E.g. I typically end up running "ag" whenever goto definition fails (ie. whenever I try finding a .cpp implementation from its .hpp declaration), so I could just put some stupid correct-90%-of-the-time fallback in there.

[unhammer]

(I'd probably use such a hook for when I e.g. change arguments to a definition as well, and have to go back to the header to change.)

[abingham]

That's an interesting idea, and I don't think it would be too much work in principle. I wonder a bit about how fine-grained it should be, though. For example, should we have a hook for goto-declaration failure, another for goto-definition failure, etc? Or should we just have a generalized "goto failed" hook (perhaps with an indicator of what kind of goto it was)?

[unhammer]

A generalized hook passed one of the same strings like "GoToDefinition" sounds good to me.

Does ycmd send an exception or what on this uhm goto fail?

[ptrv]

I think we should run a hook on any exception, passing the result to the hook function and then the user can decide what to do. The exception contains the exception type and also the message which identifies the source of the exception.

Hmm, but then you don't have the information about the command (i.e. GoToDefinition) that caused the exception.

[ptrv]

@unhammer I pushed a commit which adds a hook to run on an exception. Could you try out the PR #307?

[unhammer]

Nice, now I can at least do

(setq ycmd-after-exception-hook
      (list
       (defun ycmd-after-exception-try-ag (type res)
         (when (equal type "GoToDefinition")
           (ag (symbol-name (symbol-at-point))
               (file-name-directory (buffer-file-name)))))))

however, there are still a couple issues:

• it's a deferred call, so I may have gotten sick of waiting in the meanwhile and moved point, making ag look up the wrong symbol – is there a way to get the original point? (I couldn't find it in the exception structure at least)
• it only works on ycmd-goto-definition, not ycmd-goto, which will happily "go to" the same point as I was on. In fact, if I'm in a header file and do ycmd-goto from a symbol, then move point, I just get moved back. Maybe upstream should give an exception if a GoTo gives the same spot we're on, instead of success, though if that's unlikely to change upstream I feel like it'd make sense for ycmd.el to have a way of handling it
• if I (being impatient) do ycmd-goto several times, I get moved back several times, which is confusing and annoying – should I open an issue on cancelling existing deferred's?

[ptrv]

@unhammer Regarding the first point, I updated the PR and now we pass also buffer and point to the hook.

I am not sure what to do about point 2. Should we add some more logic to handle the case GoTo gives us back the same position?

I am also not sure regarding point 3. I could think of fixing the issue by checking whether there is an unfinished deferred request and in that case just prevent sending a new one. This would introduce a new state variable to keep track whether a GoTo is already in progress. Or should we just treat this situation as user error?

@abingham Any thoughts?

[unhammer]

Point 1 is fixed for my use at least, thanks :-)