Consolidate Scrolling actions #11660

Open
opened 2026-01-31 02:53:58 +00:00 by claunia · 3 comments
Owner

Originally created by @carlos-zamora on GitHub (Dec 2, 2020).

Description of the new feature/enhancement

After #8459, we'll have the following scroll actions:

  • scrollUp
  • scrollDown
  • scrollUpPage
  • scrollDownPage
  • scrollToTop
  • scrollToBottom

This seems to be a case of us introducing new actions when we should be introducing new arguments instead. I propose the following designs:

Design 1: 3 args

  • action: scroll
  • arguments:
    • direction: up|down
    • mode: system|page|buffer
    • rowsToScroll: <optional int>

Personally dislike that rowsToScroll only works if you're in "system" mode. But it avoids the override in Design 2.

Design 2: Override mode

  • action: scroll
  • arguments:
    • direction: up|down
    • mode: system|page|buffer|<int>

This is basically Design 1, except rowsToScroll is embedded inside mode. If we go with this approach, maybe mode should be renamed to something like "amount"?

Open to discussion. Go!

Originally created by @carlos-zamora on GitHub (Dec 2, 2020). # Description of the new feature/enhancement After #8459, we'll have the following scroll actions: - `scrollUp` - `scrollDown` - `scrollUpPage` - `scrollDownPage` - `scrollToTop` - `scrollToBottom` This seems to be a case of us introducing new _actions_ when we should be introducing new _arguments_ instead. I propose the following designs: ### Design 1: 3 args - action: `scroll` - arguments: - `direction: up|down` - `mode: system|page|buffer` - `rowsToScroll: <optional int>` Personally dislike that `rowsToScroll` _only_ works if you're in "system" mode. But it avoids the override in Design 2. ### Design 2: Override mode - action: `scroll` - arguments: - `direction: up|down` - `mode: system|page|buffer|<int>` This is basically Design 1, except `rowsToScroll` is embedded inside `mode`. If we go with this approach, maybe mode should be renamed to something like "amount"? Open to discussion. _Go!_
claunia added the Issue-FeatureArea-SettingsProduct-Terminal labels 2026-01-31 02:53:58 +00:00
Author
Owner

@DHowett commented on GitHub (Dec 3, 2020):

@zadjii-msft 2.0 -- fair?

@DHowett commented on GitHub (Dec 3, 2020): @zadjii-msft 2.0 -- fair?
Author
Owner

@zadjii-msft commented on GitHub (Dec 3, 2020):

Well, we've got an open PR here with #8459. Are we expecting to resolve this design before we merge that PR? IMO That PR seems perfectly sensible, and I'd merge it as-is. I do get the push to make it a parametrized action - it's the switchToTab1, switchToTab2 thing all over again.

What if it was

{ "command": { "action": "scroll", "direction":"up" } }, // defaults to the system number of lines
{ "command": { "action": "scroll", "direction":"up", "amount": 5 } },
{ "command": { "action": "scroll", "direction":"up", "amount": "page" } },
{ "command": { "action": "scroll", "direction":"up", "amount": "buffer" } },

Which is like design 2, but without the "system" enum value. Though, I really hate sticking the 5 in the enum.

{ "action": "scroll", "direction":"up" }, // defaults to the system number of lines
{ "action": "scroll", "direction":"up", "amount": 5 }, // defaults to lines
{ "action": "scroll", "direction":"up", "mode": "page" },
{ "action": "scroll", "direction":"up", "mode": "page", "amount": 5 }, // up 5 pages
{ "action": "scroll", "direction":"up", "mode": "buffer" },
{ "action": "scroll", "direction":"up", "mode": "buffer", "amount": 5  }, // sure, 5 buffers up. It's still the top.

Okay I think I like design 1 more

@zadjii-msft commented on GitHub (Dec 3, 2020): Well, we've got an open PR here with #8459. Are we expecting to resolve this design before we merge that PR? IMO That PR seems perfectly sensible, and I'd merge it as-is. I do get the push to make it a parametrized action - it's the `switchToTab1`, `switchToTab2` thing all over again. What if it was ```jsonc { "command": { "action": "scroll", "direction":"up" } }, // defaults to the system number of lines { "command": { "action": "scroll", "direction":"up", "amount": 5 } }, { "command": { "action": "scroll", "direction":"up", "amount": "page" } }, { "command": { "action": "scroll", "direction":"up", "amount": "buffer" } }, ``` Which is like design 2, but without the "system" enum value. Though, I really hate sticking the `5` in the enum. ```jsonc { "action": "scroll", "direction":"up" }, // defaults to the system number of lines { "action": "scroll", "direction":"up", "amount": 5 }, // defaults to lines { "action": "scroll", "direction":"up", "mode": "page" }, { "action": "scroll", "direction":"up", "mode": "page", "amount": 5 }, // up 5 pages { "action": "scroll", "direction":"up", "mode": "buffer" }, { "action": "scroll", "direction":"up", "mode": "buffer", "amount": 5 }, // sure, 5 buffers up. It's still the top. ``` Okay I think I like design 1 more
Author
Owner

@carlos-zamora commented on GitHub (Dec 3, 2020):

Well, we've got an open PR here with #8459. Are we expecting to resolve this design before we merge that PR? IMO That PR seems perfectly sensible, and I'd merge it as-is. I do get the push to make it a parametrized action - it's the switchToTab1, switchToTab2 thing all over again.

I'm ok with merging #8459. Mainly because idk when we/somebody would get around to implementing this.

What if it was

{ "command": { "action": "scroll", "direction":"up" } }, // defaults to the system number of lines
{ "command": { "action": "scroll", "direction":"up", "amount": 5 } },
{ "command": { "action": "scroll", "direction":"up", "amount": "page" } },
{ "command": { "action": "scroll", "direction":"up", "amount": "buffer" } },

So the default would be?:

{ "command": { "action": "scroll", "direction":"up", "amount": "system" } },

Which is like design 2, but without the "system" enum value. Though, I really hate sticking the 5 in the enum.

{ "action": "scroll", "direction":"up" }, // defaults to the system number of lines
{ "action": "scroll", "direction":"up", "amount": 5 }, // defaults to lines
{ "action": "scroll", "direction":"up", "mode": "page" },
{ "action": "scroll", "direction":"up", "mode": "page", "amount": 5 }, // up 5 pages
{ "action": "scroll", "direction":"up", "mode": "buffer" },
{ "action": "scroll", "direction":"up", "mode": "buffer", "amount": 5  }, // sure, 5 buffers up. It's still the top.

So the default would be?:

{ "action": "scroll", "direction":"up", "mode": "line", "amount": "system" }, 

I'm fine with either. I agree that your design 1 seems cleaner though. 😊

@carlos-zamora commented on GitHub (Dec 3, 2020): > Well, we've got an open PR here with #8459. Are we expecting to resolve this design before we merge that PR? IMO That PR seems perfectly sensible, and I'd merge it as-is. I do get the push to make it a parametrized action - it's the `switchToTab1`, `switchToTab2` thing all over again. I'm ok with merging #8459. Mainly because idk when we/somebody would get around to implementing this. > What if it was > > ```js > { "command": { "action": "scroll", "direction":"up" } }, // defaults to the system number of lines > { "command": { "action": "scroll", "direction":"up", "amount": 5 } }, > { "command": { "action": "scroll", "direction":"up", "amount": "page" } }, > { "command": { "action": "scroll", "direction":"up", "amount": "buffer" } }, > ``` So the default would be?: ```js { "command": { "action": "scroll", "direction":"up", "amount": "system" } }, ``` > Which is like design 2, but without the "system" enum value. Though, I really hate sticking the `5` in the enum. > > ```js > { "action": "scroll", "direction":"up" }, // defaults to the system number of lines > { "action": "scroll", "direction":"up", "amount": 5 }, // defaults to lines > { "action": "scroll", "direction":"up", "mode": "page" }, > { "action": "scroll", "direction":"up", "mode": "page", "amount": 5 }, // up 5 pages > { "action": "scroll", "direction":"up", "mode": "buffer" }, > { "action": "scroll", "direction":"up", "mode": "buffer", "amount": 5 }, // sure, 5 buffers up. It's still the top. > ``` So the default would be?: ```js { "action": "scroll", "direction":"up", "mode": "line", "amount": "system" }, ``` I'm fine with either. I agree that your design 1 seems cleaner though. 😊
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#11660