Do not send a symbol search request to OmniSharp if search term is shorter than "omnisharp.minFindSymbolsFilterLength" #2625
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2625 +/- ##
==========================================
+ Coverage 64.86% 65.16% +0.29%
==========================================
Files 99 99
Lines 4318 4320 +2
Branches 629 630 +1
==========================================
+ Hits 2801 2815 +14
+ Misses 1332 1317 -15
- Partials 185 188 +3
Continue to review full report at Codecov.
|
| @@ -23,10 +23,15 @@ export default class OmnisharpWorkspaceSymbolProvider extends AbstractSupport im | |||
| let options = this.optionProvider.GetLatestOptions(); | |||
| let minFilterLength = options.minFindSymbolsFilterLength > 0 ? options.minFindSymbolsFilterLength : undefined; | |||
There was a problem hiding this comment.
Sounds like you could remove the setting as well. #Closed
There was a problem hiding this comment.
the setting makes sense, it's just we evaluate it on the client rather than server
There was a problem hiding this comment.
you are right - sorry for the confusion.
| @@ -23,10 +23,15 @@ export default class OmnisharpWorkspaceSymbolProvider extends AbstractSupport im | |||
| let options = this.optionProvider.GetLatestOptions(); | |||
| let minFilterLength = options.minFindSymbolsFilterLength > 0 ? options.minFindSymbolsFilterLength : undefined; | |||
| let maxItemsToReturn = options.maxFindSymbolsItems > 0 ? options.maxFindSymbolsItems : undefined; | |||
| return serverUtils.findSymbols(this._server, { Filter: search, MinFilterLength: minFilterLength, MaxItemsToReturn: maxItemsToReturn, FileName: '' }, token).then(res => { | |||
There was a problem hiding this comment.
But FindSymbolsRequest.MinFilterLength can be removed, along with removing it on the server side, right?
There was a problem hiding this comment.
yes, I removed it from the protocol.ts, thanks for the pointer 👍
|
@filipw : let me know if you working on removing MinFilterLength from OmniSharp/omnisharp-roslyn as well - I will pick up the cleanup otherwise. thanks. |
|
we will not remove it yet to not cause a breaking change on the API, since it already shipped. I will remove it in the future, but not yet |
Instead of doing the validation on the server, we do it on the client.