Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Goto/highlight target error when moving around in the panel #1717

Open
kaste opened this issue Mar 6, 2020 · 7 comments · May be fixed by #1750
Open

Goto/highlight target error when moving around in the panel #1717

kaste opened this issue Mar 6, 2020 · 7 comments · May be fixed by #1750

Comments

@kaste
Copy link
Contributor

kaste commented Mar 6, 2020

We currently support double-clicking in the panel as a goto-command. Maybe we could reduce that to simple clicks, and then generally just follow the cursor in the panel.

Basically reducing the need for mouse interactions. We could then explore how the panel could get keyboard focus just by using the keyboard. (ctrl+ka only opens the panel without focusing it.)

Technically that means we implement on_selection_modifed for the panel view. Since the target view also implements on_selection_modified to scroll and select the correct error in the panel we need to take care of this circular triggering which will be the tricky part here.

@braver
Copy link
Member

braver commented Mar 6, 2020

Well, that is maybe tricky because you need to think about click-dragging to select text etc. Also, IIRC "we" don't support double-clicking, we just format the output in a way that makes Sublime Text support it like that. I'm hesitant to introduce behavior in our output panel that makes it different from other output panels that look similar. It intentionally mimics the output of the "find results" panel and the "build output" panels. So I'm not against researching possible improvements here, but we should fit into the eco system, or move the eco system forward.

@kaste
Copy link
Contributor Author

kaste commented Mar 6, 2020

Well, our panel is probably best-in-class. With the new "focus on one file" feature it maybe feels natural to just go through them via cursor. Of course popping up errors from other files would be surprising. Maybe on <enter> which is clearly an user action (and also missing from the built in find panel).

With the current panel it feels a bit too mousy, and that's just because the panel behaves so great in that it always shows the right information, auto-wrapped and auto-scrolled. I think I wish I could just use the keyboard more. And then introducing <n/p> shortcuts for essentially cursor navigation when you can just move the cursor...

Either on_selection_modified or <enter> I think. What would be a good method to bring the focus to the panel?

@kaste
Copy link
Contributor Author

kaste commented Apr 5, 2020

Let's brainstorm a bit:

This basically combines quick-panel features with the "output.panel".

Let's say the user opens the panel by ctrl+k+g to also focus the panel. Upon that we open the panel and put the cursor on the nearest error. The user can then use up-down to move the cursor. On cursor move and also initially, we will highlight (transiently) the error in a normal view. On ESC we will "cancel" this previewing, and focus and go to the previous view in the previous view position. On Enter we will focus the view, showing the selected error. (We probably also close our error panel.)

(This is pretty much, or should be as close as possible, how ctrl+shift+r (Goto-Symbol) works.)

If the panel is already open ctrl+k+g just focuses the panel and shows the nearest error. Just clicking into an open panel, selects that error.

Typical use-case A: You edit a file, then some import at the top of the file gets unused. You somehow get the focus to the panel as described. Use the cursor to move to the said error. Hit enter to fix it, then the builtin Go back command (e.g. alt+left) to go back to the section you were editing before.

Typical use-case B: You see that you have some errors in the file. You open and focus the panel to review some error, then hit ESC to continue working without fixing.

@braver
Copy link
Member

braver commented Apr 6, 2020

You can already keyboard navigate through stuff in the output panel (any output panel) without focusing the panel itself, via next/previous result. This works in conjunction with “go back” just like anything else. I guess I don’t understand your use cases because I think they’re already fully covered (because it’s a standard output panel)?

Maybe we could do a new command “open panel and scroll to nearest error in file” (to quickly check something relevant about what you’re writing). Or “open panel at first error in file” to start reviewing a file?

@kaste
Copy link
Contributor Author

kaste commented Apr 6, 2020

Hm, next/previous result just doesn't work really reliable (on SL3?) and the interaction with "go back" is incomprehensible. Sometimes it skips results or jumps to the next file.

In comparison the UI using the "goto symbol" (or any other quick panel) is 100%. In that: you basically open the panel, then up down to preview, enter to commit going to that location, ESC to discard.

Our own goto next/previous error command is reliable but it always commits the location to the history. Nothing we can change though (I think) because it is only one keystroke we either always commit to the history or never. (The preview feature is missing, I guess.)

@braver
Copy link
Member

braver commented Apr 6, 2020

Hm, next/previous result just doesn't work really reliable

Ok, I don't have that problem, but let's say it's there (maybe we should pin-point it and open an issue upstream).

the interaction with "go back" is incomprehensible

I think it bundles all the steps into one "edit" or something. It is weird though.

I agree that as soon as you have focus in the panel it's nice if you could do something there. It seems only natural that "Enter" would bring you to where your cursor points at.

@kaste
Copy link
Contributor Author

kaste commented Apr 6, 2020

I think I quickly(?) hack a quick-panel to see if that UX (previewing locations, commit or discard jumping, all just using the keyboard) makes a difference, esp. I would like "go back" to work better, and of course not going to the mouse at all.

@kaste kaste linked a pull request Jun 18, 2020 that will close this issue
2 tasks
kaste added a commit that referenced this issue Jul 10, 2020
Fixes #1717

Enable standard "show_panel" with "focus: True" to open the panel and focus
it.  Our toggle panel takes this arg as well and passes it through.

Generally, you *somehow* focus the panel, then `<up>`/`<down>` will move
to next or previous error transiently, t.i. without storing this move in
Sublime's navigation history.  `<enter>` will commit this new location to
said history, while `<escape>` will discard the previewing and return to the
original location.  (Overall this mimics for example the Goto Symbol quick
panel.)  Note that `<escape>` is not specially bound here, we instead
introspect or observe the standard "hide_command" with "cancel: true" command
and act upon it.

On top of this, we may close the panel only if you opened the panel with
"focus: true".  (T.i. you did not move the focus to an already open panel.)

Further notes:

If the panel has focus:

- Change the "SNAP" value to (basically) 0 to the effect that the line
  gets highlighted only if the cursor is exactly on an error.  (T.i.
  turn off the "snap to nearest value" feature.)

- Change the color of our virtual cursor to get *some* visual feedback.
  (For the panel, we choosed total control over the cursor, so basically
	the user never sees a caret. We use a _selection_ (not a region!) to
  highlight error lines; and completely remove any selection otherwise.)
  Note that we will have a normal caret now if the panel has the focus.
  That's normal behavior.
kaste added a commit that referenced this issue Jul 14, 2020
Fixes #1717

Enable standard "show_panel" with "focus: True" to open the panel and focus
it.  Our toggle panel takes this arg as well and passes it through.

Generally, you *somehow* focus the panel, then `<up>`/`<down>` will move
to next or previous error transiently, t.i. without storing this move in
Sublime's navigation history.  `<enter>` will commit this new location to
said history, while `<escape>` will discard the previewing and return to the
original location.  (Overall this mimics for example the Goto Symbol quick
panel.)  Note that `<escape>` is not specially bound here, we instead
introspect or observe the standard "hide_command" with "cancel: true" command
and act upon it.

On top of this, we may close the panel only if you opened the panel with
"focus: true".  (T.i. you did not move the focus to an already open panel.)

Further notes:

If the panel has focus:

- Change the "SNAP" value to (basically) 0 to the effect that the line
  gets highlighted only if the cursor is exactly on an error.  (T.i.
  turn off the "snap to nearest value" feature.)

- Change the color of our virtual cursor to get *some* visual feedback.
  (For the panel, we choosed total control over the cursor, so basically
	the user never sees a caret. We use a _selection_ (not a region!) to
  highlight error lines; and completely remove any selection otherwise.)
  Note that we will have a normal caret now if the panel has the focus.
  That's normal behavior.
kaste added a commit that referenced this issue Aug 3, 2020
Fixes #1717

Enable standard "show_panel" with "focus: True" to open the panel and focus
it.  Our toggle panel takes this arg as well and passes it through.

Generally, you *somehow* focus the panel, then `<up>`/`<down>` will move
to next or previous error transiently, t.i. without storing this move in
Sublime's navigation history.  `<enter>` will commit this new location to
said history, while `<escape>` will discard the previewing and return to the
original location.  (Overall this mimics for example the Goto Symbol quick
panel.)  Note that `<escape>` is not specially bound here, we instead
introspect or observe the standard "hide_command" with "cancel: true" command
and act upon it.

On top of this, we may close the panel only if you opened the panel with
"focus: true".  (T.i. you did not move the focus to an already open panel.)

Further notes:

If the panel has focus:

- Change the "SNAP" value to (basically) 0 to the effect that the line
  gets highlighted only if the cursor is exactly on an error.  (T.i.
  turn off the "snap to nearest value" feature.)

- Change the color of our virtual cursor to get *some* visual feedback.
  (For the panel, we choosed total control over the cursor, so basically
	the user never sees a caret. We use a _selection_ (not a region!) to
  highlight error lines; and completely remove any selection otherwise.)
  Note that we will have a normal caret now if the panel has the focus.
  That's normal behavior.
kaste added a commit that referenced this issue Aug 21, 2020
Fixes #1717

Enable standard "show_panel" with "focus: True" to open the panel and focus
it.  Our toggle panel takes this arg as well and passes it through.

Generally, you *somehow* focus the panel, then `<up>`/`<down>` will move
to next or previous error transiently, t.i. without storing this move in
Sublime's navigation history.  `<enter>` will commit this new location to
said history, while `<escape>` will discard the previewing and return to the
original location.  (Overall this mimics for example the Goto Symbol quick
panel.)  Note that `<escape>` is not specially bound here, we instead
introspect or observe the standard "hide_command" with "cancel: true" command
and act upon it.

On top of this, we may close the panel only if you opened the panel with
"focus: true".  (T.i. you did not move the focus to an already open panel.)

Further notes:

If the panel has focus:

- Change the "SNAP" value to (basically) 0 to the effect that the line
  gets highlighted only if the cursor is exactly on an error.  (T.i.
  turn off the "snap to nearest value" feature.)

- Change the color of our virtual cursor to get *some* visual feedback.
  (For the panel, we choosed total control over the cursor, so basically
	the user never sees a caret. We use a _selection_ (not a region!) to
  highlight error lines; and completely remove any selection otherwise.)
  Note that we will have a normal caret now if the panel has the focus.
  That's normal behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants