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

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

Open
wants to merge 1 commit 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.

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.

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