Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

command to swap visible files between panes per issue #13061 #13150

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SteveMieskoski
Copy link

per issue #13061 , I added a function to swap visible files between panes, keybinding to Ctrl + Alt + F , and a preference with a disabled default state.
The function itself is currently located within MainViewManager.js with the command and preference manager registration located in ViewCommandhandlers.js as it seemed the logical place. There are three tests covering the basic functionality, the case where the file to move already exists in the destination pane, and where only one pane contains open/visible files. In the latter case I just call moveView.

If its felt this should not exist in the core, but in an extension I don't think that will be much of an issue. I'm interested in any Comments and/or Remarks.

…ggestion: Add a keybind command to swap the files currently open in upper and lower pane.
src/config.json Outdated
@@ -20,7 +20,7 @@
"extension_url": "https://s3.amazonaws.com/extend.brackets/{0}/{0}-{1}.zip",
"linting.enabled_by_default": true,
"build_timestamp": "",
"healthDataServerURL": "https://healthdev.brackets.io/healthDataLog"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert this change, it happens automatically after npm install but shouldn't be committed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the change, and that's good to know, thanks.

@zaggino
Copy link
Contributor

zaggino commented Mar 5, 2017

Looks pretty good to me, I should be able to test tomorrow.

activeFileInactiveView = inactivePane.getViewForPath(activeFile.fullPath);
inactiveFileActiveView = activePane.getViewForPath(inactiveFile.fullPath);

if (inactiveFileActiveView) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could probably be a method like _moveView is

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petetnt I looked into moving most of what is now in MainViewManager onto the pane prototype (see below), and I can push up the commit including it, however, with this setup I can't seem to get the active pane to consistently stay in place. It flips from one pane to the other every third swap. I've tried promises and a few other ideas, but Maybe I'm missing something obvious. Any insights are welcome.

    Pane.prototype.swapPaneContent = function(oppositeFile) {
        var self = this,
            oppositePane = self.id === FIRST_PANE ? SECOND_PANE : FIRST_PANE;

        if(!oppositeFile){
            var file = self.getCurrentlyViewedFile();
            MainViewManager._moveView( self.id, oppositePane.id, file , 0);
        } else {
            if (self.getViewForPath(oppositeFile.fullPath)) {
                CommandManager.execute(Commands.FILE_OPEN, {
                    fullPath: oppositeFile.fullPath,
                    paneId: self.id
                });
            } else {
                CommandManager.execute(Commands.CMD_ADD_TO_WORKINGSET_AND_OPEN, {
                    fullPath: oppositeFile.fullPath,
                    paneId: self.id
                });
            };
        };
    };
    function swapPaneContent() {

        var activePaneId = getActivePaneId(),
            inactivePaneId = activePaneId === FIRST_PANE ? SECOND_PANE : FIRST_PANE,
            activePane = _getPane(activePaneId),
            inactivePane = _getPane(inactivePaneId),
            activeFile = activePane.getCurrentlyViewedFile(),
            inactiveFile = inactivePane.getCurrentlyViewedFile();

            inactivePane.swapPaneContent(activeFile);   // addition 
            activePane.swapPaneContent(inactiveFile);  // addition
    };

Copy link
Contributor

@zaggino zaggino Mar 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this rewrite ... Pane.prototype calling MainViewManager methods doesn't seem right. Maybe @petetnt can specify little more what he meant, but looking at the code I prefer the original version.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my idea was something in line with the rewrite but just calling the method with pane, file arguments instead of adding it to the prototype.

The original code is fine by me too if you think so too, you call @SteveMieskoski 👍

@zaggino
Copy link
Contributor

zaggino commented Mar 6, 2017

I like the code, I like the feature, works quite well, but two things:

  1. Why do I have to enable the feature in preferences?
  2. This breaks Add a feature to toggle between panes in the core #10555 #12853 in a way that if I use ctrl+alt+f to switch panes, alt+w stops to work until I mouse-click to the second pane, can you have a look why is that?
  3. If we have alt+w to jump cursor between panes, it'd seem logical (at least to me) to have something+w to exchange them

@SteveMieskoski
Copy link
Author

@zaggino having it as a default disabled preference was a mention in the original issue, and wouldn't take much to implement/remove so I went in that direction. As such there isn't any necessary reason for it to be so. I'll look into why alt+w stops working, and while I'm at it also change the keybindings to something+w because I agree it makes more sense.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants