Skip to content
This repository was archived by the owner on Apr 1, 2020. It is now read-only.

Provide binding to jump to next/previous error #2514

Closed
psxpaul opened this issue Aug 20, 2018 · 5 comments
Closed

Provide binding to jump to next/previous error #2514

psxpaul opened this issue Aug 20, 2018 · 5 comments

Comments

@psxpaul
Copy link
Contributor

psxpaul commented Aug 20, 2018

Oni Version: 0.3.6
Neovim Version (Linux only): n/a
Operating System: MacOS 10.12

Issue:
When editing a file that is supported by a configured language server, I would like to be able to jump to the next/previous highlighted error. There doesn't appear to be any way to do this currently, unless I'm missing something.

Looking into the code, I see 3 ways of implementing this:

  • add all diagnostic errors to the Quickfix List, so they can be navigated with [q and ]q
  • add all diagnostic errors to the Location List, so they can be navigated with [l and ]l
  • add a new entry to browser/src/Services/GlobalCommand, so that custom bindings can be added by the user

The Quickfix and Location List approaches have the downside of potentially overwriting anything already in those lists. I know that workspace searches and find-all-references searches populate the Quickfix list, so that's probably not ideal for many users.

I'm also curious if there's a default binding we should set for this. JetBrains IDEs like WebStorm and IntelliJ use F2/Shift-F2 for these bindings, while VSCode and VisualStudio use F8/Shift-F8.

Expected behavior:
Press a key combo (e.g. ), and the cursor would jump to the next highlighted error.

Actual behavior:
No such binding is available or documented

Steps to reproduce:

  • Open a Typescript file
  • type in some gibberish
  • verify the nonsensical line is highlighted
  • jump to a different spot in the file
  • how to quickly jump back to error?
@oni-bot
Copy link

oni-bot bot commented Aug 20, 2018

Hello and welcome to the Oni repository! Thanks for opening your first issue here. To help us out, please make sure to include as much detail as possible - including screenshots and logs, if possible.

@badosu
Copy link
Collaborator

badosu commented Aug 20, 2018

Quickfix is usually used for errors on a build step, this may make sense for some language server operations (e.g. c++ compilation, etc).

Location list makes sense for syntax errors and quick checks.

I agree with avoiding both, perhaps a built-in error list. This error list can be reused for modernizing the quickfix and location list later on (also leveraging the new layer api). It can also be another entry on the sidebar.

]e and [e are for swapping lines on unimpaired.vim (embedded by default for Oni) but a similar mnemonic would be interesting.

@psxpaul
Copy link
Contributor Author

psxpaul commented Aug 20, 2018

Thanks for the feedback @badosu. I'm also leaning toward the 3rd implementation I listed. I have a prototype locally just for jumping between errors. It should be possible to later add a GUI element for listing errors across the entire project.

I'll probably implement this without a default binding, since most IDEs seem to have different shortcuts for this.

@badosu
Copy link
Collaborator

badosu commented Aug 20, 2018

I'll probably implement this without a default binding, since most IDEs seem to have different shortcuts for this.

Kudos for that 👍, although default FX keybindings (also C-PgUp, C-PgDwn and similar) are usually not an issue for experienced vimmers (which tend to concentrate keybindings in the middle of the keyboard).

@akinsho
Copy link
Member

akinsho commented Aug 20, 2018

@psxpaul @badosu I agree re. avoiding the quickfix and loclists for this as I think that could open a can of worms but might be interesting to revisit once they are externalised I think the third option sound great 👍, there should be a list in the/a redux store of all the errors and the associated lines its currently being used to render markers on the buffer scroll bar but could be also be used for this

akinsho pushed a commit that referenced this issue Aug 29, 2018
This PR adds `oni.editor.nextError` and `oni.editor.previousError` commands, as discussed in #2514. I have mainly tested this with Typescript.
@psxpaul psxpaul closed this as completed Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants