Add commands to jump to previous/next error #2526
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2526 +/- ##
==========================================
+ Coverage 43.57% 44.43% +0.85%
==========================================
Files 351 351
Lines 14186 14260 +74
Branches 1846 1862 +16
==========================================
+ Hits 6182 6336 +154
+ Misses 7767 7697 -70
+ Partials 237 227 -10
Continue to review full report at Codecov.
|
| @@ -45,6 +50,78 @@ export const activate = ( | |||
| const popupMenuPrevious = popupMenuCommand(() => menuManager.previousMenuItem()) | |||
| const popupMenuSelect = popupMenuCommand(() => menuManager.selectMenuItem()) | |||
|
|
|||
| const gotoNextError = async () => { | |||
There was a problem hiding this comment.
Could these functions be moved into their own files in the Diagnostics directory maybe Diagnostics/navigateErrors.ts so the global commands isn't where they are implemented and just import them here?
There was a problem hiding this comment.
Thanks for the feedback! I moved this to browser/src/Services/Diagnostics/navigateErrors.ts, but that might be confusing since there's already a browser/src/Services/Diagnostics.ts. I can move that file inside the new browser/src/Services/Diagnostics directory if you want.
There was a problem hiding this comment.
we could make the already existing Diagnostics.ts => browser/src/Services/Diagnostics/index.ts that way if there are future helpers etc. they can go in the same dir plus you wont have to change any imports since as index all the imports of the file should work as is
| const currentFileErrors = getAllErrorsForFile(activeBuffer.filePath, errors) | ||
| const currentPosition = activeBuffer.cursor | ||
|
|
||
| if (!currentFileErrors || currentFileErrors.length === 0) { |
There was a problem hiding this comment.
currentFileErrors.length === 0 could be !currentFileErrors
| } | ||
| } | ||
|
|
||
| activeBuffer.setCursorPosition( |
There was a problem hiding this comment.
This should also be awaited as above I think?
| lastError = error | ||
| } | ||
|
|
||
| activeBuffer.setCursorPosition(lastError.range.start.line, lastError.range.start.character) |
There was a problem hiding this comment.
not sure if this is async but its likely to be in which case does it need to be awaited since its not being returned to be awaited elsewhere
| * Test that you can jump to the next/previous error | ||
| */ | ||
| import * as assert from "assert" | ||
| import * as Oni from "oni-api" |
|
going to merge this now if you're ready @psxpaul? Thanks again 🎉 |
|
@psxpaul the commands work like a dream very nice work ⭐️, would you be able to add them to the documentation somewhere under the config section so other users can find them. |
|
@psxpaul Do you mind adding an entry on the wiki for this? I fear the lack of default keybindings may make this feature invisible to users. |
|
I've added some bindings to this page, but I'm not sure if that's the best place for it. |
|
Seems good enough, thanks! Just noticed me and @Akin909 posted at the same time |
|
Awesome! Been wanting this for a while, very nice work - great test coverage too @psxpaul ! 💯 |
This PR adds
oni.editor.nextErrorandoni.editor.previousErrorcommands, as discussed in #2514. I have mainly tested this with Typescript.