Skip to content

Add "Move Tab to a New Window" in tab context menu #509

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

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

Conversation

Palollo
Copy link

@Palollo Palollo commented Jan 6, 2025

And open new windows in the default bounds, not in the stored bounds, avoiding that confusing behavior when the new window is created in the same exact position of the previous one.
The stored bounds are used for the first instance.

@derceg
Copy link
Owner

derceg commented Jan 6, 2025

I think adding this functionality is a good idea, though I think the implementation should be different.

I recently added a MultipleWindowsPerSession command line flag. When Explorer++ is invoked with that flag, as follows:

explorer++.exe --enable-features MultipleWindowsPerSession

there can be more than one top-level window in a session. Then, when Explorer++ next starts, each of the previous windows will be restored, rather than just the first one.

Hosting multiple windows in one process should ultimately work a lot better:

  • At the moment, data is saved by each process, so if you have multiple windows open by multiple processes, exactly which process saves data isn't clear. It's also possible (though unlikely) that two processes will both try to save data at the same time and the final result will be a mix of both.

  • Data isn't shared between processes. So, if you create a bookmark in one process, it won't appear in the second process. Sharing data between processes is possible, but a lot more difficult. Managing everything in a single process is a lot easier.

  • Storing everything in a single process will also make it easier to move tabs between existing windows.

  • As you noted, if you create a new window, that window will use the previously saved settings. Which might or might not match the currently saved settings, depending on when the settings were saved.

If you run Explorer++ with the MultipleWindowsPerSession flag, there will be a New Window item added to the File menu. When you select that item, a new window will be created in the same process, at an offset. See Explorerplusplus::OnNewWindow.

By adding some similar code, you could create a new window with a specific tab. For example, if you took the code for Explorerplusplus::OnNewWindow and added something like:

initialData.tabs = { { .pidl = pidlOfExistingTab } };
initialData.selectedTab = 0;

then that should result in a new window being created with that one specific tab.

So, I think this functionality should be gated by the MultipleWindowsPerSession flag. That is, the "Move Tab to a New Window" menu item should only appear if the MultipleWindowsPerSession feature is enabled. You can see an example of how to do that in Explorerplusplus::InitializeMainMenu. You can then create a new window, with a specific tab, by using something like the code above.

At the moment, MultipleWindowsPerSession is still disabled by default, because some things don't work as expected when it's enabled. For example, the Search Tabs dialog only shows tabs in the current window, whereas it should show tabs in all windows. But, by and large, most things just work and the intention is to enable this feature by default once the remaining issues are ironed out.

@derceg
Copy link
Owner

derceg commented Jan 6, 2025

Actually, what might work better would be to add your changes, excluding the changes to Explorerplusplus::CreateMainWindow. Then, I can update Explorerplusplus::OpenDirectoryInNewWindow to create a new window in-process if the MultipleWindowsPerSession feature is enabled. Or you can do that as part of this pull request if you want.

But I don't think adjusting the main window position is useful, given that the eventual plan is to only ever have a single process and there's already code to offset the window position in that case.

Palollo added 2 commits April 28, 2025 09:46
And open new windows in the default bounds, not in the stored bounds,
avoiding that confusing behavior when the new window is created
in the same exact position of the previous one.
The stored bounds are used for the first instance.
…ssion

# Conflicts:
#	Explorer++/Explorer++/Explorer++.cpp
@Palollo Palollo force-pushed the move_tab_to_new_window branch from 6dc0af3 to 6cfa47b Compare April 28, 2025 08:30
@Palollo
Copy link
Author

Palollo commented Apr 28, 2025

OK, I have rebased to synchronize with master and added condition to avoid conflicts in setting the bounds of new window when the feature MultipleWindowsPerSession is enabled.

So, I think this functionality should be gated by the MultipleWindowsPerSession flag. That is, the "Move Tab to a New Window" menu item should only appear if the MultipleWindowsPerSession feature is enabled.

I use multiple windows in my daily work, I know all the four points you mention, they don't usually bother me. So I want the menu item appear in the normal mode until the MultipleWindowsPerSession feature becomes enabled by default (I tried it and has too many bugs).

Anyway, feel free to modify anything you want in this PR-branch, you have edit permissions.
You can even copy the parts you like to your own commit, closing the PR without merge. It's up to you don't worry.

By the way, let me suggest: new features usually are managed by git branches, not with "secret" parameters ;)

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

Successfully merging this pull request may close these issues.

2 participants