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

Communicate between Porcupine instances #1215

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Communicate between Porcupine instances #1215

wants to merge 17 commits into from

Conversation

rdbende
Copy link
Collaborator

@rdbende rdbende commented Nov 3, 2022

This breaks the poppingtabs plugin, so some --new-window command line option is needed.
Would be also good to have a checkbox in the settings window to turn off this feature.

Copy link
Owner

@Akuli Akuli left a comment

Choose a reason for hiding this comment

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

This is awesome! I think we should make a new release before merging this, so that we can add --new-window or whatever the option will be in a separate PR.

def open_files(files: Iterable[str]) -> None:
tabmanager = get_tab_manager()
for path_string in files:
if path_string == "-":
Copy link
Owner

Choose a reason for hiding this comment

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

Presumably this doesn't work when opening from tk send. It would be nice if we could fix it (here or in a separate PR), so that you could do

$ some long command | porcu -

to show the output of the command in the currently running Porcupine instance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, it doesn't

@Akuli
Copy link
Owner

Akuli commented Nov 3, 2022

Would be also good to have a checkbox in the settings window to turn off this feature.

I don't think this is needed, at least not for now. We can always add it later if someone needs it.

@rdbende
Copy link
Collaborator Author

rdbende commented Nov 4, 2022

Ohh shit. Looks like this will be an X-only feature.

image
The send command doesn't even exists on Windoze, and winfo interps returns nothing 🤦🏼‍♀️

On OSX with aqua send exists, although it does nothing (this suggests me that it could work with an X windowing system), and winfo interps returns only the current Tcl interpeter, but not others.

@Akuli
Copy link
Owner

Akuli commented Nov 4, 2022

You can revert #786.

@Moosems
Copy link
Contributor

Moosems commented May 16, 2023

I already implemented this in DIP (yes, I know you can't open multiple instances yet but its set up to handle it) and feel like it will be a super awesome thing to have in Porcupine.

@rdbende
Copy link
Collaborator Author

rdbende commented May 16, 2023

I don't think that's how we want to do it. Especially because most of us use Porcupine on Linux.

@Moosems
Copy link
Contributor

Moosems commented May 16, 2023

What do you mean by "Communicate between Porcupine instances" then?

@rdbende
Copy link
Collaborator Author

rdbende commented May 16, 2023

"Allow only a single Porcupine instance" and #92

@Moosems
Copy link
Contributor

Moosems commented May 16, 2023

"single Porcupine instance" and "Porcupine instances" are antonyms. "single... instance" means there's only one instance but the "s" in "Porcupine instances" infers multiple instances working together. If there's only "a single Porcupine instance", are you saying it needs to speak with itself? You said using it as an optional command but that would negate the need for this. All this to say: what?

@rdbende
Copy link
Collaborator Author

rdbende commented May 16, 2023

It means that when you try to open a file in Porcupine at the command line or by double clicking in the file manager, it doesn't create a completely new window, but sends it to an existing Porcupine process which will hamdle it.
So "communicating between Porcupine instances" means that it forwards files to be opened by an existing Porcupine instance.

@rdbende rdbende self-assigned this May 16, 2023
@Akuli
Copy link
Owner

Akuli commented May 17, 2023

Ideally you could still have multiple Porcupine instances running. It just wouldn't be the thing that happens when you open files by double-clicking each one individually, instead they would all go to the same instance/process/window.

porcupine/_state.py Outdated Show resolved Hide resolved
porcupine/_state.py Outdated Show resolved Hide resolved
@Akuli
Copy link
Owner

Akuli commented Aug 10, 2023

Nice to see some progress on this :)

@rdbende
Copy link
Collaborator Author

rdbende commented Nov 6, 2023

Oh man, what I just did to all the tests

@Moosems
Copy link
Contributor

Moosems commented Nov 6, 2023

Uh oh man what'd you do 🙄😂?

@rdbende
Copy link
Collaborator Author

rdbende commented Nov 6, 2023

Idk. Something with the cache directory being elsewhere on GH actions. The tests work locally.

@ethical-haquer
Copy link
Contributor

Any updates on this?

@rdbende
Copy link
Collaborator Author

rdbende commented Apr 3, 2024

It works. The test need fixing, but otherwise it's good, AFAIR.

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.

4 participants