Move window operations

[coolwanglu]

Implement the commands like ^WH.
I found it not easy to write spec about moving panes, I'm not familiar with the atom API, especially those about creating and identifying panes, any suggestions?

[maxbrunsfeld]

@coolwanglu this is cool. I'd really like to have some specs for this before merging, but I'm not sure where to point you for examples. Stand by. @nathansobo do you have any advise for testing this? Also, is there any way to accomplish this without constructing a PaneAxis manually as @coolwanglu is doing here?

[coolwanglu]

@maxbrunsfeld Right, PaneAxis stuffs are really hacks. I didn't find them in the API doc, and I referred to Atom's source code.

[nathansobo]

@maxbrunsfeld For testing, you should probably just use atom.workspace and do a state based test, unless I'm missing something.

I wonder if you could achieve this via normal API's by creating a new pane in the desired location, moving all the items, then destroying the old pane. Any reason that wouldn't work?

[coolwanglu]

@nathansobo The problem is exactly to create a pane at the desired position. Say the current layout is something like

{
  PaneAxis : 'vertical'
  children: [Pane1, Pane2, Pane3]
}

and the result after moving should be

{
  PaneAxis: 'horizontal'
  children: [Pane2,
    {
       PaneAxis: vertical
       children: [Pane1, Pane3]
    }]
}

In this case I don't know how to create the new Pane which is a sliding of the old vertical PaneAxis.
Seems that the normal API only allows us to create a new Pane as a sibing of an existing Pane.

[nathansobo]

Ah, I see. You're right. This definitely expands the surface area of our panes API.

@maxbrunsfeld I think we should think through how we could extend our official API to support this before merging this into an officially maintained package. Do you agree?

@coolwanglu if you'd like to think through a more ideal official API for this we could entertain a PR on atom/atom. What if we exposed the axes as part of the API and provided split|Up|Down|Left|Right methods on the axis objects? That would enable you to create a pane relative to an axis without needing to construct the axis yourself.

[coolwanglu]

@nathansobo Right, if spiltUp (and also other directions) is defined as create a new child pane at the top (for vertical axis) or create a new sibling pane on top (for horizontal axis), the code will be much simpler.

Also we need functions to retrieve the root pane/paneaxis, and to check whether it is a pane or axis.

[nathansobo]

@coolwanglu And if you call splitRight on a vertical axis, it would create a new containing horizontal axis automatically. The split... naming is starting to feel awkward when applied to axes however. Seems like addPaneAbove, addPaneBelow, addPaneToLeft, addPaneToRight might be clearer and apply better to both axes and panes. The split... method on axes would be a bit misleading since calling splitUp on a vertical axis wouldn't actually split anything... it would just add a new child at the top.

Would you be interested in opening a PR on master with these changes? If you agree about the naming change, we would need to leave the old split... methods on pane but steer people toward the addPane... versions.

[coolwanglu]

@nathansobo I'm afraid that I might not have enough time recently. I will come back later and implement this if nobody has done so.

[nathansobo]

@coolwanglu Cool, thanks a lot!

[bronson]

Any hope on getting the calls into Atom? If not, is there a downside to merging this as-is for now? It can always be updated when Atom provides the calls.

Also, if I can get an answer on how to write tests for #723, I'd volunteer to write tests for this one too. They look like they'd be pretty similar.

[t9md]

For reference.
I also implemented this feature ctrl-w HJKL with my paner package.
First I tried simply replace root PaneAxis like this PR do, but didn't work.
It seem to OK for visually when first moving pane to very-righ/top/bottom/left, but following split operation won't reflect view(I mean further split operation won't split Pane any more).

Although I need to investigate further, the approach I finally succeeded to achieve this feature is to copy whole original root PaneAxis and its child and use this copied paneAxis as child of newPaneAxis.

So, as same reason, I don't think this PR work as-is, does someone tried this PR?

Here is memo after struggling to implement this feature.

copying original Root paneAxis

copyRoot using copyPaneAxis recursively.

This feature is one of top feature I used to use in Vim.
Hope this feature is implemented vim-mode somehow.

[bronson]

Yeah, not working well would be a pretty big downside! I haven't tried this patch -- it didn't look like it was very mergeable yet. I notice the lack of move-window operations every day... guess it's time to get off my butt and set up paner. (edit: works great! but this functionality still belongs in vim-mode).

[coolwanglu]

I tried to tested if this patch still works for the latest version of vim-mode, and I found that:

• I have accidentally removed my repo. Oops!
• Several APIs about Pane have been integrated into Atom

So I guess the better way is to implement this in Atom as window:move-pane-to-very-left etc.

Maybe this PR should be closed?

[t9md]

Several APIs about Pane have been integrated into Atom

What API are you saying integrated?
I want to know since I also want to implement this better way.

By the way when you created this PR? Could you split pane for pane which is moved to very far position further.
In my experience, If I simply replace paneAxis without copying it, it wont reflect further split action to UI.

[coolwanglu]

@t9md For example, this one: https://github.com/coolwanglu/vim-mode/blob/4aba8f61eb768968fc7e74f767efa41cca63a757/keymaps/vim-mode.cson#L84
The file for pane operations in vim-mode has been removed due to this.

I'm not sure about your question, I don't remember if I have tested that or not. Sorry.

I guess it's best to implement this directly in Atom, we don't even need new APIs that way.

[t9md]

@coolwanglu Thanks.
I agree Atom core to provide API or command to make this operation more simpler.
Currently core still lack of API to achieve this operation. So need to improve further.

[lee-dohm]

As stated in the README, this package is no longer maintained and is deprecated. We recommend that people use the vim-mode-plus package instead. Because of this, we are archiving this repository and closing all issues and pull requests. Thanks very much for your support and contributions!