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

Fix 1279 #1284

Merged
merged 6 commits into from
May 14, 2021
Merged

Fix 1279 #1284

merged 6 commits into from
May 14, 2021

Conversation

benoit-pierre
Copy link
Member

@benoit-pierre benoit-pierre commented Apr 21, 2021

Summary of changes

Remove the process lock, switch to a custom controller object for both ensuring that only one instance of Plover is running, and sending commands to that instance. Reuse the focus command to restore and focus the main window when attempting to launch a second time.

Closes #1279.

Pull Request Checklist

  • Changes have tests
  • News fragment added in news.d. See documentation for details

@user202729
Copy link
Member

Note.

  • Now Plover is remote-controllable (how much?), is it controllable from other machines in the same LAN?
  • Yet another breaking change. Conflict somewhat with output plugin.
  • Aside: I also made a plugin (plover-websocket-server-2) to remotely control Plover, but I guess it suffer from the above problem more; besides I'm probably the only one to use it.

@benoit-pierre
Copy link
Member Author

* Now Plover is remote-controllable (how much?), is it controllable from other machines in the same LAN?

Only from the same machine: Unix socket on Linux/macOS, and named pipe on Windows.

@user202729
Copy link
Member

Note: what happens if Plover is not properly exited and the file /tmp/plover_socket is not deleted?

On the other hand the old lock thing might have the same issue.

@benoit-pierre
Copy link
Member Author

Note: what happens if Plover is not properly exited and the file /tmp/plover_socket is not deleted?

In case of hard crash (segfault, abort), it's possible that the socket won't be properly deleted.

On the other hand the old lock thing might have the same issue.

Not on Windows, since it was using an OS primitive.

@user202729
Copy link
Member

user202729 commented Apr 27, 2021

It is possible to do something similar to vim -- include the PID information somewhere, and if the PID is dead assume that the socket is left after a crash (and just force delete it).

Although there must be some library for that already...?

@benoit-pierre
Copy link
Member Author

It is possible to do something similar to vim -- include the PID information somewhere, and if the PID is dead assume that the socket is left after a crash (and just force delete it).

Changed to something simpler: if we can't connect to send the focus command, clean-up any stray Unix socket and restart.

@benoit-pierre
Copy link
Member Author

OK, I think this is ready for review.

Copy link
Member

@morinted morinted left a comment

Choose a reason for hiding this comment

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

Looks good to me. On Mac, when Plover is in system tray, the focus puts Plover in focus (menu bar updates) but no main window appears. This is consistent with how quicktime works so I think it's okay.

Not used anymore, and support was dropped in setuptools>=52.0.0.
Remove the process lock, switch to a custom controller object for both
ensuring that only one instance of Plover is running, and sending
commands to that instance. Reuse the focus command to restore and focus
the main window when attempting to launch a second time.
@benoit-pierre benoit-pierre merged commit dfec830 into openstenoproject:master May 14, 2021
@benoit-pierre benoit-pierre deleted the fix_1279 branch May 14, 2021 20:26
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.

Raise and focus running instance of Plover when launching it while running
3 participants