-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Make reveal position smoother #16311
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
Make reveal position smoother #16311
Conversation
When debugging, the current position always is centered, which leads to a lot of jumping around during stepping. The auto option for reveal position is much smoother. Signed-off-by: Florian Richter <[email protected]>
| if (Position.is(selection)) { | ||
| editor.cursor = selection; | ||
| editor.revealPosition(selection); | ||
| editor.revealPosition(selection, { vertical: 'auto' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the behavior for the breakpoint case is improved, but I'm a little hesitant to put this change here in the EditorManager, since it means it will change the behavior of all callers providing positions as selection (but also not those providing ranges, for better or worse). Would it make sense to put the relevant code in the handler for the stop event, instead, to more narrowly target the case of interest? Can you think of other cases where the jumpiness of centering is inappropriate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In further thought, I agree. When selection search results, clicking on a breakpoint in the breakpoint list, selection an object in the call hierarchy or clicking on an error message in the terminal it might make sense to center the view, because an actual jump is expected when using these features.
I'll see, if I can add option and handle this in the stop event.
Signed-off-by: Florian Richter <[email protected]>
| if (Position.is(selection)) { | ||
| editor.cursor = selection; | ||
| editor.revealPosition(selection, { vertical: 'auto' }); | ||
| editor.revealPosition(selection, { vertical: options?.revealOption ?? 'center' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like enabling this parameter to be customized by callers. Now that it is customizable, I think it would make sense to apply the same customization to revealRange below.
Signed-off-by: Florian Richter <[email protected]>
What it does
When debugging, the current position is always actively centered. This causes the visible part of the document to jump around for each debug step. The auto option for reveal position leads to a much smoother experience.
How to test
Follow-ups
Attribution
Contributed on behalf of MVTec Software GmbH
Review checklist
Reminder for reviewers