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

API Request: Move Tab #8234

Closed
rebornix opened this issue Jun 28, 2016 · 16 comments
Closed

API Request: Move Tab #8234

rebornix opened this issue Jun 28, 2016 · 16 comments
Assignees
Labels
api feature-request Request for new features or functionality workbench-tabs VS Code editor tab issues
Milestone

Comments

@rebornix
Copy link
Member

rebornix commented Jun 28, 2016

  • VSCode Version: 1.3.0-Insiders
  • OS Version: OSX

Per #6605 , we already exposed APIs to move active editors across groups. And we do support drag and drop tabs so I'm thinking can we expose tabs moving functionalities as API, then people can move tabs by commands.

This will benefit Vim extensions as moving tabs is a useful often-used command. For now only the last four commands can be implemented with existing APIs.

:tabs         list all tabs including their displayed windows
:tabm 0       move current tab to first
:tabm         move current tab to last
:tabm {i}     move current tab to position i+1

:tabn         go to next tab
:tabp         go to previous tab
:tabfirst     go to first tab
:tablast      go to last tab

@bpasero @kieferrm

@rebornix
Copy link
Member Author

I'd like to leverage workbench.action.showEditorsInGroup for :tabs but seems it's still not exposed publicly yet.

@bpasero bpasero added feature-request Request for new features or functionality workbench labels Jun 28, 2016
@bpasero
Copy link
Member

bpasero commented Jun 28, 2016

👍
related: #8185

@bpasero
Copy link
Member

bpasero commented Jul 14, 2016

@egamma @sandy081 I could look into this, since VIM is on the plan for July, do you think we should have it?

@jrieken any objections adding more API commands like that to our API? This command imho does not make much sense as a global command (e.g. in F1) so I would only add it as parameterized API command.

@rebornix to clarify, this is not about moving a tab to another editor group, only move it within a group right?

@rebornix
Copy link
Member Author

@bpasero yes you are right! It's all about moving tabs within a group. I would be really happy to see this feature go with July 👍

@jrieken
Copy link
Member

jrieken commented Jul 14, 2016

Yeah, make sense to have that as a command tho I am not sure how you want to parameterise them. Do you have the ViewColumn and then from: number and to: number where the number values represent a tab?

Like executeCommand('moveTab', ViewColumn.One, { from: 1, to: 3 })

Problem I see is that we don't expose in what tab an editor is, only in what view column

@bpasero
Copy link
Member

bpasero commented Jul 14, 2016

I was about to ask the same question. @rebornix would the use case be to move the active tab that has keyboard focus or any tab (active and inactive) opened anywhere?

@rebornix
Copy link
Member Author

@bpasero , moving current (active) tab is enough. Besides, Editor Group is a VS Code concept so moving active to other groups is not a Vim thing. All Vim users care about is just re-ordering tabs IMHO.

@bpasero
Copy link
Member

bpasero commented Jul 14, 2016

Ok since this is very specific I also suggest an explicit command like:

  • executeCommand('moveActiveTab', to: <enum>)

Where the to <enum> argument is BEGINNING, END, LEFT, RIGHT. With that we avoid the problem that we do not have API to find the current index of the tab.

@rebornix would you need a command to move 2 indexes to the left or right or is this enum enough?

@jrieken
Copy link
Member

jrieken commented Jul 14, 2016

What would this command do when tabs aren't enabled? Should we strive for a command that handles both cases?

@sandy081
Copy link
Member

Why not using string constants for arguments instead of enums. We cannot know what a number means while consuming in

  • Extensions (If we do not expose it)
  • Key bindings

The cursor commands which I am writing are string based. I think we should be consistent here either string or enums.

@bpasero
Copy link
Member

bpasero commented Jul 15, 2016

@sandy081 @rebornix I suggest you guys add this command based on the pattern you find best suitable for VIM. It is very easy to move a tab to another index:

You can get hold of the IEditorGroupService in the commands accessor and call moveEditor(input: IEditorInput, from: Position, to: Position, index?: number): void to move a tab.

Use let activeEditor = IEditorService.getActiveEditor() (note this is another service, still some debt here to align both) to get the currently active editor. This editor can be NULL if there is no active one!!!

As for the arguments:

  • input: use activeEditor.input
  • from: use activeEditor.position
  • to: use activeEditor.position to move within the same group
  • index: see below how to compute the index and do the right thing

You can use IEditorGroupService.getStacksModel().groupAt(activeEditor.position) to get the IEditorGroup of the editor. With this interface you can find out the index of it IEditorGroup.indexOf(activeEditor.input) as well as find out the number of opened tabs in the group (IEditorGroup.count).

The place where we add and document API commands is: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/api/node/extHostApiCommands.ts#L212

@bpasero bpasero modified the milestones: July 2016, Backlog Jul 15, 2016
@bpasero bpasero assigned rebornix and sandy081 and unassigned bpasero Jul 15, 2016
@bpasero
Copy link
Member

bpasero commented Jul 15, 2016

If tabs are disabled, maybe you should actually move the editor between the groups? The API allows this easily and in order to find out that tabs are enabled or disabled, you could just go to the configuration service:

const useTabs = !!configuration.workbench.editor.showTabs

@rebornix
Copy link
Member Author

rebornix commented Jul 20, 2016

@bpasero didn't notice you asked me a quesiton about this feature. Moving tabs by specifying the index instead of <enum> is easier for implementing Vim commands:

:tabm 0       move current tab to first   -> executeCommand('moveActiveTab', to: 1)
:tabm         move current tab to last     -> executeCommand('moveActiveTab', to: 999)
:tabm {i}     move current tab to position i+1  ->  executeCommand('moveActiveTab', to: i+1 )

It's also easy to implement this command as we just need to pass the index to moveEditor(input: IEditorInput, from: Position, to: Position, index?: number): void.

When the Tab feature is disabled, we can use to index to decide which editor group that the tab should be moved to.

Sounds good to you all?

@sandy081
Copy link
Member

sandy081 commented Jul 22, 2016

@rebornix Implemented moveActiveEditor command, following is the documentation:

Usage:

  • Command Id: moveActiveEditor
  • Arguments: {to: $Position, by: $By, value: number}

Position

  • first
  • last
  • left
  • right
  • center
  • position

By

  • tab
  • group

Default: If tabs are enabled tab otherwise group

In Tabs mode:

  • first: Moves to first position in the current group
  • last: Moves to last position in the current group
  • left: Moves to left by given amount in the current group. Default is 1.
  • right: Moves to right by given amount in the current group. Default is 1.
  • center: Moves to center of the group.
  • position: Moves to the amount position in the current group. Default is 1. Note: value is One based.

In Groups mode:

  • left | first: Moves to Left group
  • right | last : Moves to Right group
  • center: Moves to center group
  • position: Moves to the amount group. Default position is 1.

1: Left, 2:Center, 3:Right

Vim Usage for moving current active editor to:

:tabm 0     first               ->  executeCommand('activeEditorMove', to: 'first')
:tabm        last               ->  executeCommand('activeEditorMove', to: 'last')
:tabm {i}   position        ->executeCommand('activeEditorMove', to: 'position' value: i + 1)

If you track the # of tabs, then you can just use executeCommand('activeEditorMove', to: 'position' value: i + 1) with appropriate index.

sandy081 added a commit that referenced this issue Jul 22, 2016
@sandy081
Copy link
Member

sandy081 commented Jul 25, 2016

@rebornix Please note following changes to the API to make it neat and consistent with existing

  • Renamed the command name to moveActiveEditor
  • Renamed the argument property from amount to value
  • Positions are made 1 based. First position is 1 and so on...

Please adopt to these changes

@rebornix
Copy link
Member Author

@sandy081 thanks, I've updated with the new signature. They worked.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api feature-request Request for new features or functionality workbench-tabs VS Code editor tab issues
Projects
None yet
Development

No branches or pull requests

4 participants