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

windows: Implement dock menus #17352

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

JunkuiZhang
Copy link
Contributor

The implementation of the dock menu on Windows relies on the single instance feature. If we could implement this single instance feature at the gpui layer, it would make the dock menu on Windows much more seamless.

As I mentioned earlier, when clicking on Zed's dock menu in Windows, Windows always opens a new app instance. The current implementation works by detecting if another instance is running when the dock menu is clicked. If another instance is found, the dock menu information is sent to that instance, which then performs the corresponding action, and the current instance will quit.

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Sep 4, 2024
Comment on lines 319 to +328

fn main() {
let start_time = std::time::Instant::now();

#[cfg(target_os = "windows")]
{
use zed::windows_only_instance::*;
register_zed_identifier();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible that we check single instance at the very beginning?

@@ -361,11 +368,15 @@ fn main() {
}
}

let args = Args::parse();
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want this behavior, the sqlite database we use can't support more than one instance. If we remove this, I'd be happy to merge this feature :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, but I didn't quite understand your point. Could you please explain how let args = Args::parse(); is related to the SQLite database? I mean it is simply handling the arguments passed to Zed, like zed.exe --some_args, am I wrong?

Copy link
Contributor

@mikayla-maki mikayla-maki Nov 22, 2024

Choose a reason for hiding this comment

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

I'm sorry, I misclicked the line. I meant to select the new_instance field of 'Args', which I think means 'create a new Zed process', and if there are two Zed processes talking to the same sqlite DB, things will break because sqlite requires serialized updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see.

Sorry for the confusion here! The new_instance here doesn't refer to creating a new instance of zed itself, but rather to the startup arguments passed to an instance.

Handling the Dock Menu on Windows is a bit more complicated. When you click a button on the Dock Menu, like New Window, Windows spawns a new instance of zed, similar to command::new("zed.exe").spawn(). To differentiate which button the user clicked, we need to pass some arguments to this new instance, such as zed.exe --new_instance="workspace::NewWindow".

When the new instance starts, the previously merged single instance feature detects an already running instance through the check_single_instance() function. It also notices the Dock Menu argument workspace::NewWindow attached to the new instance and forwards this argument to the existing instance before exiting itself. The flow looks something like this:

if !check_single_instance() {
    if let Some(dock_menu_args) = args.new_instance {
        send_to_existing_instance(dock_menu_args);
    }
    println!("Already running!");
    return;
}

Then, the existing instance, after receiving the Dock Menu argument workspace::NewWindow from the closing instance, begins executing the function associated with New Window.

Since English isn't my first language, I often struggle with naming things, so I simply used new_instance here. Sorry for the confusion caused!

@mikayla-maki
Copy link
Contributor

By the way, I think the single instance feature has been merged. What does this PR need to be shipped or closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants