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

Improve heuristic for detecting terminal output #91

Open
Misterio77 opened this issue Dec 18, 2024 · 9 comments
Open

Improve heuristic for detecting terminal output #91

Misterio77 opened this issue Dec 18, 2024 · 9 comments
Labels
enhancement New feature or request
Milestone

Comments

@Misterio77
Copy link

Misterio77 commented Dec 18, 2024

Hi there! Thanks for this lovely project, it's by far the tool that best handles my use cases, and I'm super thankful to you for picking up maintenance and adding so many nice features.

I'm not sure if there's something wrong in my setup, but it seems that the terminal output detection gives false positives and thus, in some situations, does not launch a new terminal for apps that have Terminal=true.

For example: running handlr launch text/plain through hyprland binds does not launch a terminal for neovim, but binding handlr launch text/plain | cat does. The same happens if I run it through wofis run menu. Both are surely not attached to a terminal (so no visible neovim for me), and yet are detected as such by handlr.

Is it alright if I look into improving this somehow? Just wanted make sure it's not something that only happens to me, haha.

Or maybe the issue is with Hyprland after all. I wonder if other people have the same issues in other WMs. I'll see if I can test it with sway later today.

Thanks!

@Misterio77 Misterio77 changed the title Improve heuristic for detecting terminal Improve heuristic for detecting terminal output Dec 18, 2024
@Anomalocaridid
Copy link
Owner

Hi there! Thanks for this lovely project, it's by far the tool that best handles my use cases, and I'm super thankful to you for picking up maintenance and adding so many nice features.

Always glad to hear I'm helping someone :)

I'm not sure if there's something wrong in my setup, but it seems that the terminal output detection gives false positives and thus, in some situations, does not launch a new terminal for apps that have Terminal=true.

For example: running handlr launch text/plain through hyprland binds does not launch a terminal for neovim, but binding handlr launch text/plain | cat does. The same happens if I run it through wofis run menu. Both are surely not attached to a terminal (so no visible neovim for me), and yet are detected as such by handlr.

What version of handlr-regex are you using? IIRC there was a similar problem in the past so it might be solved by updating it if you're not already at the latest version.

Also what version of Hyprland are you using? I also use Hyprland and use a keybind similar to yours, but I currently do not have the same problem.

Is it alright if I look into improving this somehow? Just wanted make sure it's not something that only happens to me, haha.

By all means, go ahead.

@Misterio77
Copy link
Author

Also what version of Hyprland are you using? I also use Hyprland and use a keybind similar to yours, but I currently do not have the same problem.

Oh, that's interesting. I think it worked at some point for me, but I don't use the binding too much so I don't remember. when it happened.

I'm currently running handlr-regex 0.12.0 and Hyprland 0.45.2, how about you?

@Anomalocaridid
Copy link
Owner

Anomalocaridid commented Dec 18, 2024

I am also running handlr-regex 0.12.0 and Hyprland 0.45.2.

What terminal emulator and shell are you using?

Does this happen with other terminal-based programs?

Do you have x-scheme-handler/terminal set in ~/.config/mimeapps.list?

Also, what is the exact Hyprland bind that you are using?

@Misterio77
Copy link
Author

Misterio77 commented Dec 18, 2024

Thanks for the help!

I am also running handlr-regex 0.12.0 and Hyprland 0.45.2.

Ah, I guess we're both on nixos-unstable? Haha

What terminal emulator and shell are you using?

Alacritty and fish.

Does this happen with other terminal-based programs?

Yep, I've tried setting the Default Applications for text/plain to both nvim.desktop and helix.desktop. The bind breaks for both (but works for non-terminal apps as expected).

Do you have x-scheme-handler/terminal set in ~/.config/mimeapps.list?

Yep! Here's how it looks like:

[Added Associations]
x-scheme-handler/terminal=Alacritty.desktop

[Default Applications]
text/plain=nvim.desktop
x-scheme-handler/terminal=Alacritty.desktop
x-scheme-handler/http=org.qutebrowser.qutebrowser.desktop;firefox.desktop
x-scheme-handler/https=org.qutebrowser.qutebrowser.desktop;firefox.desktop

Also, handlr launch x-scheme-handler/terminal works as expected.

Also, what is the exact Hyprland bind that you are using?

Here you go:

bind=SUPER,e,exec,/nix/store/mjgm7094vfl2mgbmagv4gfmgn6amk4dm-handlr-regex-0.12.0/bin/handlr launch text/plain

As mentioned, appending a | cat to it makes the bind work as expected.

@Anomalocaridid
Copy link
Owner

I can replicate the bug using alacritty. Do you have term_exec_args set in ~/.config/handlr.toml? If so, what is it set to?

@Misterio77
Copy link
Author

Hey

Yep, I do (but I think it was autogenerated by handlr). It's set to -e:

term_exec_args = '-e'

Which is the right setting for alacritty.

I added some logging, and it seems that config.terminal_output is true when running the hyprland bind I mentioned.

It's weird that you can reproduce it with alacritty but not other terminals, though. I wonder if that's two separate issues? Can you try running it with this patch?

diff --git a/src/common/desktop_entry.rs b/src/common/desktop_entry.rs
index 6cf050a..644d472 100644
--- a/src/common/desktop_entry.rs
+++ b/src/common/desktop_entry.rs
@@ -129,6 +129,7 @@ impl DesktopEntry {
             exec.extend_from_slice(&args);
         }

+        std::fs::write("/tmp/handlr.log", format!("Requires terminal={}, Has terminal output={}", self.terminal, config.terminal_output)).unwrap();
         // If the entry expects a terminal (emulator), but this process is not running in one, we
         // launch a new one.
         if self.terminal && !config.terminal_output {

@Misterio77
Copy link
Author

Misterio77 commented Dec 19, 2024

It seems that both hyprland and sway forward their stdin and stdout to programs launched in them through bindings. Launching Hyprland with exec Hyprland >/dev/null fixes the problem.

Maybe this is out of scope for handlr to handle (heh)? Perhaps handlr could have a flag (maybe something like --terminal) to override the terminal_output heuristic?

@Anomalocaridid
Copy link
Owner

Yep, I do (but I think it was autogenerated by handlr). It's set to -e:

term_exec_args = '-e'

Which is the right setting for alacritty.

I added some logging, and it seems that config.terminal_output is true when running the hyprland bind I mentioned.

It's weird that you can reproduce it with alacritty but not other terminals, though.

If term_exec_args was properly set, then I was not, in fact, actually replicating the issue because it works fine on my system with it being properly set. Sorry, that was an oversight on my part.

It seems that both hyprland and sway forward their stdin and stdout to programs launched in them through bindings. Launching > Hyprland with exec Hyprland >/dev/null fixes the problem.

Strange. That is probably what is throwing off the terminal detection.

Although I wonder why it is happening on your system and not mine.

Maybe this is out of scope for handlr to handle (heh)? Perhaps handlr could have a flag (maybe something like --terminal) to override the terminal_output heuristic?

I agree, it seems more like a Hyprland-related issue at this point.

Perhaps handlr could have a flag (maybe something like --terminal) to override the terminal_output heuristic?

Good idea. I'll make sure to implement it in the near future.

Another workaround you might consider in the meantime would be launching the terminal manually like this:

handlr launch x-scheme-handler/terminal -- handlr launch text/-plain

@Misterio77
Copy link
Author

Sorry, that was an oversight on my part.

Don't worry about that! Thanks a lot for taking some time to help me out :))

Although I wonder why it is happening on your system and not mine.

Hmmm, how you are you launching Hyprland? I'm personally using greetd+regreet

Good idea. I'll make sure to implement it in the near future.

I'll see if I can get a quick patch going in the next couple of days

Thanks a lot, and happy holidays!

@Anomalocaridid Anomalocaridid added this to the v0.13.0 milestone Jan 10, 2025
@Anomalocaridid Anomalocaridid added the enhancement New feature or request label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants