Editor Scroll #681
Editor Scroll #681
Conversation
| @@ -1466,6 +1510,28 @@ class CommandCenterScroll extends BaseCommand { | |||
| } | |||
|
|
|||
| @RegisterAction | |||
| class COmmandTopScroll extends BaseCommand { | |||
There was a problem hiding this comment.
Minor casing issue - there's an uppercase O in CommandTopScroll
There was a problem hiding this comment.
Ooops, I've put too much time on shift ...
|
I've mentioned before that I dislike updating the view outside of our updateView function, though in the past I never really drew a hard line. However, I've actually started to take steps to minimize this behavior within my multi-cursor-mode branch, and I think it would be prudent to begin to draw the line and say we want to do that everywhere. (This has a zillion benefits, least of which include separation of concerns, really simplifying macros and speeding up . down the line.) The way I'd suggest to do something like this would be that for actions that can adjust the view, make simple interfaces like |
|
@johnfn ha I was expecting this feedback on this :) I do agree that any side effect should happen in a centralized place like updateView but sometimes it's difficult. The reason that Code won't provide a pure command for us to call (I mentioned quite a bit times) is a pure command without any side effect may be not useful to other callers and way too Vim specific. The top 1st command/API that conflicts with your principle (making commands pure/ putting view updating all in viewUpdate) is I'm not against your proposal and actually I do want to make it this way in VSCodeVim but we do have limitations. I'll read your multi-cursor PR to grab ideas from you and update this PR accordingly. |
|
For the pure, just scroll and do nothing else commands, I think it's possible! But the other ones will be more difficult, you're right. Kind of a bummer. |
|
This would also fix #210 |
rebornix
force-pushed
the
ScrollPages
branch
from
13348cd
to
3d9237e
Compare
|
@johnfn @jpoon just followed Johnfn's suggestions, postpone Code commands which have side effect to view layout and only run them when we trigger our inhouse Look forward to your comments on this. |
| @@ -609,16 +635,49 @@ class CommandDeleteIndentInCurrentLine extends BaseCommand { | |||
| } | |||
| } | |||
|
|
|||
| class CommandCtrlE extends CommandEditorScroll { | |||
| keys = ["<C-e>"]; | |||
| to = "down"; | |||
There was a problem hiding this comment.
Can we create enums for to and by values?
There was a problem hiding this comment.
Sure, that's definitely necessary.
| @@ -1088,6 +1100,15 @@ export class ModeHandler implements vscode.Disposable { | |||
|
|
|||
| vscode.window.activeTextEditor.setDecorations(this._caretDecoration, rangesToDraw); | |||
|
|
|||
| if (this.vimState.postponedCodeViewChanges.length > 0) { | |||
| @@ -33,6 +33,11 @@ export enum VimSpecialCommands { | |||
| Dot | |||
| } | |||
|
|
|||
| export class ViewChange { | |||
| public name: string; | |||
There was a problem hiding this comment.
nit: maybe rename this to command? As that'll be inline with how it's used in executeCommand.
|
@jpoon comments addressed! |
| /** | ||
| * Positions in the view for cursor move command. | ||
| */ | ||
| export const CursorMovePosition = { |
There was a problem hiding this comment.
You could actually just use string union types here, which I like better (stronger typechecking).
type CursorMovePosition = 'left' | 'right' | 'up' | 'down' | ...
There was a problem hiding this comment.
yes, string union type can easily validate string value assignment but if we use that, then we still need to write plain text string while initialization in action.ts, which is what we want to avoid, right?
rebornix
force-pushed
the
ScrollPages
branch
from
0e8b82b
to
6589e90
Compare
Add editor scroll commands. The trick in this PR is disabling view updating for editor scrolling as Code already does it for us, re-updating the view will lead to unexpected result of layout redraw.
This PR should fix #672, #673, #675, #676 .