add options for jumping to next/previous problem #135736
Conversation
There was a problem hiding this comment.
Thanks for starting this.
Tho, we cannot change the behavior just like that for all our users. Traversing diagnostics by severity (error, warning, info, hint) was a conscious decision. Please understand that users like the current behavior just like you dislike it. Therefore, this needs to be behind a user-setting.
|
|
pohzipohzi
changed the title
|
@jrieken thank you for the feedback. I've updated to the PR to make the sort order configurable, how does it look? I'll try to update the documentation accordingly if this works |
| }, | ||
| 'problems.sortOrder': { | ||
| 'description': 'TODO', | ||
| 'type': 'array', |
There was a problem hiding this comment.
It is better to use an enum because there is a fixed option of picks, take this as a sample:
There was a problem hiding this comment.
what enum values would you propose, looking at the rest of the comments perhaps just 2: severityFirst and startPositionFirst?
There was a problem hiding this comment.
I think just severity and position is enough
| ) { | ||
| if (URI.isUri(resourceFilter)) { | ||
| this._resourceFilter = uri => uri.toString() === resourceFilter.toString(); | ||
| } else if (resourceFilter) { | ||
| this._resourceFilter = resourceFilter; | ||
| } | ||
|
|
||
| const sortOrder = this._configService.getValue<string[]>('problems.sortOrder'); |
There was a problem hiding this comment.
You should be retrieving a single string (not array) which is one of the enum values.
| } | ||
| switch (sortOrder[i]) { | ||
| case 'resource': | ||
| res = compare(a.resource.toString(), b.resource.toString()); |
There was a problem hiding this comment.
Sorting by resource cannot be configurable and must always be checked first. The resource comparing is done for notebook cells (which is multiple resources/document within a single logical notebook editor) and you don't want F8 to jump back and forth between cells.
| res = Range.compareRangesUsingStarts(a, b); | ||
| break; | ||
| case 'ends': | ||
| res = Range.compareRangesUsingEnds(a, b); |
There was a problem hiding this comment.
Does comparing by ends really make sense?
|
Made the enum changes, I'll fill in the docs if this looks alright 🙂 |
|
I am seeing two
|
|
Thanks for pointing that out. The JSON schema default value didn't match the enum set. I have pushed a fix |
466d041 update distro ccdd1bf update distro 5be3575 Remove unnecessary regexp escape (for #137166) bacf6ed smoke - properly create screenshots and stop instances (#137220) c33965a Fix terminal dnd feedback ddd2f59 Fix terminal detach session 5243c40 #136424 add to proposed api f76b133 Don't wait for 4s orphan check for just revived processes 4a7cd94 tweak for microsoft/vscode#135736 (comment) git-subtree-dir: code git-subtree-split: 466d041
This PR fixes #135249
Default behaviour:
After setting
"problems.compareOrder": "position":