Skip to content
This repository has been archived by the owner on Apr 14, 2022. It is now read-only.

Navigation and tooltip on self #251

Merged
merged 18 commits into from
Oct 16, 2018
Merged

Navigation and tooltip on self #251

merged 18 commits into from
Oct 16, 2018

Conversation

MikhailArkhipov
Copy link

@MikhailArkhipov MikhailArkhipov commented Oct 13, 2018

Fixes #22

  • Provide tooltip on self that is from the class definition
  • Goto definition on self navigates to the class definition
  • Move hover tests to a separate class
  • Fix issues in various tests that did not actually load files into the server. Opening is not enough if more than a single file is involved.

@@ -878,6 +885,9 @@ class bcd(abc):
var fobUri = TestData.GetTempPathUri("fob.py");
var oarUri = TestData.GetTempPathUri("oar.py");
using (var server = await CreateServerAsync(PythonVersions.LatestAvailable3X)) {
await server.LoadFileAsync(fobUri);
Copy link
Contributor

@AlexanderSher AlexanderSher Oct 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server.DidOpenTextDocument calls the same AddFileAsync as LoadFileAsync (ProjectFiles.GetEntry is a simple dictionary lookup):
https://github.com/Microsoft/python-language-server/blob/39a17b1a8aefed9ee18d904ceff2d3c447f0f060/src/LanguageServer/Impl/Implementation/Server.cs#L192

So if these loads change anything in test results, it is not because of the code flow, but because of the timings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, maybe calling EnqueueItem twice really changes something, but that isn't expected either.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is known problem with files being analyzed while other files are still being added. I am going to get my AnalysisQueue.Start/Stop change and see if that works better.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AnalysisQueue.Start/Stop works OK, no need to double load/open


public event EventHandler AnalysisStarted;
public event EventHandler AnalysisComplete;
public event EventHandler AnalysisAborted;
public event EventHandler<UnhandledExceptionEventArgs> UnhandledException;

public int Count => _ppc.Count;
public void Start() => _queueEnabled.Set();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we can replace pair of Start and Stop with IDisposable Pause using CountdownDisposable from RTVS: https://github.com/Microsoft/RTVS/blob/master/src/Common/Core/Impl/Disposables/CountdownDisposable.cs

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I did it in tests, will move to main line.


await server.WaitForCompleteAnalysisAsync(CancellationToken.None);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, these changes make tests pass, but when we make changes to LanguageServer so that it doesn't load all the files in the root directory and subdirectories, these tests will become irrelevant to the product behavior, cause there will be no waiting.

Copy link
Author

@MikhailArkhipov MikhailArkhipov Oct 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even then the analysis will still have to be completed fully on the loaded set and importing mechanism will be loading files that are referenced, even if they are not open. Basicalle, to make that case work we may need to use AST to fugure out what else to load besides the file being opened.

@MikhailArkhipov MikhailArkhipov mentioned this pull request Oct 15, 2018
@MikhailArkhipov MikhailArkhipov deleted the 22 branch October 17, 2018 00:45
jakebailey pushed a commit to jakebailey/python-language-server that referenced this pull request Nov 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants