Add option of having a target text in "Go to implementation" / find implementations responses #13
Conversation
|
Some implementation comments: If there exists a better way to get a line with a given line number Also, I'd like to hear comments on displaying a larger "target text" I'm currently writing a port for the emacs text editor, and it looks |
|
Thanks for this. You are totally right. It's just that I implemented Find Usages after Goto Implementation (so there was no QuickFix object at that time). I think I'd actually prefer it if you overwrote the original API and I can fix up any vimscript that needs changing. Nobody else is using this feature, so it's no big deal. Better than 2 APIs that do the same job IMO. If there's a better way to get the line by given line number, I don't know it. What you have seems pretty reasonable to me, although I'm no NRefactory guru.... I just fumble around the code until I get something working usually :) Not quite sure that I understand what you mean by larger "target text"....Vim's quickfix window is very limited indeed. Pretty sure that you aren't going to improve upon it. It just seems to format things however it likes. Good to hear that you are writing an Emacs port.. let me know how you get on (and if there's anything you'd like to be improved upon). This project started life as a huge hack and has slowly got into better shape. |
|
Also, any chance of a couple of tests? |
|
Hmmm just wondered if you knew about this setting? https://github.com/nosami/Omnisharp/blob/master/plugin/OmniSharp.vim#L21 |
Now returns IEnumerable<QuickFix> instead of a GotoImplementationResponse.
This will play nicer with testing. I had problems with testing json with DeserializeJson<IEnumerable<QuickFix>> so I decided to try with this route instead.
Running this does not work for me on mono. It fails with the following exception: mono --debug /usr/lib/nunit/nunit-console.exe OmniSharp.Tests.dll -run OmniSharp.Tests.GotoImplementation NUnit version 2.5.10.0 Copyright (C) 2002-2010 Charlie Poole. Copyright (C) 2002-2004 James W. Newkirk, Michael C. Two, Alexei A. Vorontsov. Copyright (C) 2000-2002 Philip Craig. All Rights Reserved. Runtime Environment - OS Version: Unix 3.2.0.40 CLR Version: 4.0.30319.1 ( 2.10.8.1 (Debian 2.10.8.1-1ubuntu2.2) ) ProcessModel: Default DomainUsage: Single Execution Runtime: Default Selected test(s): OmniSharp.Tests.GotoImplementation .Loading test.cs F Tests run: 1, Errors: 1, Failures: 0, Inconclusive: 0, Time: 0.798994 seconds Not run: 0, Invalid: 0, Ignored: 0, Skipped: 0 Errors and Failures: 1) Test Error : OmniSharp.Tests.GotoImplementation.GotoImplementationTests.Should_find_usages_in_same_file System.NullReferenceException : Object reference not set to an instance of an object at OmniSharp.Parser.BufferParser.ParsedContent (System.String editorText, System.String filename) [0x0000d] in /home/mika/koodi/OmniSharpServer/OmniSharp/Parser/BufferParser.cs:19 at OmniSharp.GotoImplementation.GotoImplementationHandler.FindDerivedMembersAsQuickFixes (OmniSharp.GotoImplementation.GotoImplementationRequest request) [0x00000] in /home/mika/koodi/OmniSharpServer/OmniSharp/GotoImplementation/GotoImplementationHandler.cs:28 at OmniSharp.Tests.GotoImplementation.GotoImplementationTests.Should_find_usages_in_same_file () [0x0007b] in /home/mika/koodi/OmniSharpServer/OmniSharp.Tests/GoToImplementation/GoToImplementation.cs:41 at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&) at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0
|
Heh, great! :) I replaced the old API with the new one. Now the API call returns a I also refactored a bit by adding a QuickFixResponse base class. I also took a shot at adding a test. This, unfortunately, was I then added a unit test, but while that compiled, it crashes at I tried running all tests in the suite, but many/most of them crashed |
| @@ -6,5 +10,34 @@ public class QuickFix | |||
| public int Line { get; set; } | |||
| public int Column { get; set; } | |||
| public string Text { get; set; } | |||
|
|
|||
| public Location ConvertToLocation() { | |||
There was a problem hiding this comment.
Don't need to worry about breaking the API. I'd rather have a consistent QuickFix response, which in turn means I can reuse the client side code for quickfixes :)
|
Thanks for that. I'll merge this in as soon as I get chance to assess / fix up the vimscript. I can't say that I've ever ran the tests on mono, I'll check that out. Thanks for trying! |
|
Just a quicks head's up - I changed both GotoImplemenation and FindUsages to return a Quickfix response. |
There currently exists no way to receive the "target text" for a
findimplementations / GotoImplementationRequest. This is inconsistent,
given that a findusages / FindUsagesRequest provides them.
FindUsagesResponse "target texts" are currently provided using the
QuickFix class, whereas the GotoImplementationResponse uses a Location
class (which does not have a "Text" member).
This change provides a way to get a listing of QuickFix responses for
a GotoImplementationRequest. It should break no existing code, since
this feature is provided as a new API called
"findimplementationsasquickfixes".