Introduce configuration for "Go to Symbol in Workspace" #2487
Conversation
|
|
|
✓ Prints the download status to the logger as Downloading package 'somePackage' (4 KB)..........Failed Building locally without any of my changes produces the same failure for me. The build is flaky? Is there a way to kick off a retry? |
|
Looking forward to this 👍 Question: Should "minFindSymbolsFilterLength" wait for server respone at all? Extension could skip sending request and return empty array immediately. IIRC Visual Studio by default waits for one character, I assume this should be the default value here as well, instead of zero. |
|
|
@akshita31 Thoughts on #2487 (comment) ? |
Codecov Report
@@ Coverage Diff @@
## master #2487 +/- ##
=========================================
- Coverage 64.75% 64.4% -0.36%
=========================================
Files 90 90
Lines 4123 4130 +7
Branches 593 593
=========================================
- Hits 2670 2660 -10
- Misses 1283 1295 +12
- Partials 170 175 +5
Continue to review full report at Codecov.
|
|
@dmgonch That is the first time I have seen that test being flaky. For now I merged master and the test passes. |
…om/dmgonch/omnisharp-vscode into feature/minFindSymbolsFilterLength
|
Strange, the code coverage report claim that coverage % lowered only in the files that I didn't change (unless I'm misreading it). Please advise how I should proceed. |
… always populated with latest settings
There was a problem hiding this comment.
@dmgonch You could add a test to the "Option Provider" here: https://github.com/OmniSharp/omnisharp-vscode/blob/b989062320acae41de758cebae89f82ea80cd938/test/unitTests/OptionObserver/OptionProvider.test.ts just to be sure that the changes are passing through.
Looks good to me(will be great if we add the test). Thanks for your contribution!
|
@akshita31 Since I didn't make any changes the "Option Provider" class itself, could you please clarify what tests you would like to see added to OptionProvider.test.ts? Thanks for signing off but these changes might need to be reworked based on the outcome of OmniSharp/omnisharp-roslyn#1284 (comment) |
|
@dmgonch The optionProvider has a function call "GetLatestOptions" which returns the latest options when the options are changed, so you might add a test there that changes your options and checks if on invoking the latest options you are getting the required changes. But yes you are right you didn't make any change to that class so doesnt make sense to add the test there. Thanks for clarifying. |
|
@rchande : do you still have any concerns about this change? |
Codecov Report
@@ Coverage Diff @@
## master #2487 +/- ##
=========================================
- Coverage 65.24% 65.1% -0.14%
=========================================
Files 97 97
Lines 4212 4221 +9
Branches 603 605 +2
=========================================
Hits 2748 2748
- Misses 1293 1302 +9
Partials 171 171
Continue to review full report at Codecov.
|
|
@rchande : the test seems to be flaky. Here is your recent build that had the same issue https://travis-ci.org/OmniSharp/omnisharp-vscode/builds/427774928. What is best way to deal with this issue for this change? |
|
@rchande , @akshita31: Rebuild failed with another error seeming unrelated to my change "The command "gulp 'vsix:release:package'" failed". What should I do to get pass the failure? Still cannot find a how to restart a build manually. |
|
@dmgonch I have seen this problem when there is some error when I execute "npm run compile". Can you check if that is the case here ? |
|
@akshita31 : fixed - thanks! Any suggestions on the coverage failure? |
|
@akshita31 : I might be reading the report incorrectly but it looks like all lines that I added are covered, no? https://codecov.io/gh/OmniSharp/omnisharp-vscode/compare/43b6fb24458787e7bd65f0c5b7df129aa5f89832...1c8828975ab778dba8acee66014b6347c48b5eef/diff |
|
I'm guessing it might be because OmnisharpWorkspaceSymbolProvider doesn't have any UTs at all at the moment: https://codecov.io/gh/OmniSharp/omnisharp-vscode/compare/43b6fb24458787e7bd65f0c5b7df129aa5f89832...1c8828975ab778dba8acee66014b6347c48b5eef/diff |
|
@dmgonch You are right, the coverage failure is probably because we do not have any integration/unit tests that cover the WorkspaceSymbolProvider. But the coverage is not a mandatory test, so I will go ahead and merge your PR now. If you are interested you can create a PR to add an integration test for the same,otherwise I will take it up as a separate work item myself. Also, you can add a change log update to both the repositories. Thanks for your contribution. |
Allow configuring number of chars user must to type in for "Go to Symbol in Workspace" command to return any results (default is 0 to preserve existing behavior). The reasoning here is that when lots of projects are loaded by OmniSharp it makes little sense to return all symbols discovered. This change should largely address these issues as well: OmniSharp/omnisharp-roslyn#1243, #1808.
Additionally allow configuring max number of items returned by "Go to Symbol in Workspace" command. Similarly to the above in a big repo returning pages and pages of items in most cases will be wasteful. But default is 0 which is indicates no limit to preserve existing behavior.