[BUG] :grep uses wrong selection when a `toolsclient` exists

[losnappas]

Version of Kakoune

vgit-e74a3ac6a3bad1b74af71aa0bfdacb41ffcb7355

Reproducer

define-command -override ide %{
  rename-client main
  new rename-client tools
  set-option global toolsclient tools
}

:ide<ret> to spawn a toolsclient

Now select something in main client and do :grep<ret>, and what'll happen is that it's going to use the selection in toolsclient instead of from the main client.

Outcome

It's going to use the $kak_selection from toolsclient instead of from the main client.

You can verify by set-option global grepcmd echo

Expectations

It should use selection from active window

Additional information

• nixos
• sway
• foot terminal
• Didn't attempt with kak -n, couldn't figure how to properly source the autoloads then

The fifo command is wrapped in evaluate-commands -try-client %opt(toolsclient), which I believe is causing this. https://github.com/mawww/kakoune/blob/9358c62bf2c26f5604ddffb7b41d55ff89a9e51f/rc/tools/grep.kak#L14

[krobelus]

confirmed. We need to grab the selection before we switch to the toolsclient

[losnappas]

I think a -try-client or similar on fifo itself would be helpful, that's where I typically want the fifo but not the evaluation context

[krobelus]

Didn't attempt with kak -n, couldn't figure how to properly source the autoloads then

the following command works, but only after adding require-module grep. Maybe we should fix KakBegin hooks defined after KakBegin to fire immediately, like we did for ModuleLoaded.

kak -n -e 'source rc/tools/grep.kak; source rc/tools/fifo.kak; source rc/tools/jump.kak; require-module grep'

I think a -try-client or similar on fifo itself would be helpful, that's where I typically want the fifo but not the evaluation context

Good idea; if it works without being too complicated then we should probably do that.

Here's a fix for the immediate problem:

From abf6d56203efe91ea48b9e487f5c4831eb0a9ba0 Mon Sep 17 00:00:00 2001
From: Johannes Altmanninger <aclopte@gmail.com>
Date: Tue, 26 Nov 2024 14:49:48 +0100
Subject: [PATCH] Fix grepcmd arguments being evaluated in toolsclient context

Commit ef18d3cbf (rc make/grep: evaluate makecmd in calling context,
use eval semantics again, 2024-07-31)
fixed a regression in make.kak but didn't bother
to fix a similar regression in make.kak.

Evaluate grepcmd and selection in the calling context again.
While at it, fix the case where ":grep" is used on a selection
that starts with "-".

Also, get rid of the double evaluation of makecmd, for consistency
with grepcmd, and in case makecmd contains unbalanced braces.
---
 rc/tools/grep.kak | 47 +++++++++++++++++++++++++++--------------------
 rc/tools/make.kak | 10 ++++++----
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/rc/tools/grep.kak b/rc/tools/grep.kak
index 9d97ffddd..3db8ca6a3 100644
--- a/rc/tools/grep.kak
+++ b/rc/tools/grep.kak
@@ -11,26 +11,33 @@ define-command -params .. -docstring %{
     All optional arguments are forwarded to the grep utility
     Passing no argument will perform a literal-string grep for the current selection
 } grep %{
-    evaluate-commands -try-client %opt{toolsclient} %{
-        fifo -name *grep* -script %exp{
-            trap - INT QUIT
-            if [ $# -eq 0 ]; then
-                case "$kak_opt_grepcmd" in
-                ag\ * | git\ grep\ * | grep\ * | rg\ * | ripgrep\ * | ugrep\ * | ug\ *)
-                    set -- -F "$kak_selection"
-                    ;;
-                ack\ *)
-                    set -- -Q "$kak_selection"
-                    ;;
-                *)
-                    set -- "$kak_selection"
-                    ;;
-                esac
-            fi
-            %opt{grepcmd} "$@" 2>&1 | tr -d '\r'
-        } -- %arg{@}
-        set-option buffer filetype grep
-        set-option buffer jump_current_line 0
+    evaluate-commands -save-regs gs %{
+        set-register g %opt{grepcmd}
+        set-register s %val{selection}
+        evaluate-commands -try-client %opt{toolsclient} %{
+            fifo -name *grep* -script %{
+                trap - INT QUIT
+                grepcmd=$1
+                selection=$2
+                shift 2
+                if [ $# -eq 0 ]; then
+                    case "$grepcmd" in
+                    ag\ * | git\ grep\ * | grep\ * | rg\ * | ripgrep\ * | ugrep\ * | ug\ *)
+                        set -- -F -- "$selection"
+                        ;;
+                    ack\ *)
+                        set -- -Q -- "$selection"
+                        ;;
+                    *)
+                        set -- -- "$selection"
+                        ;;
+                    esac
+                fi
+                eval "$grepcmd \"\$@\"" 2>&1 | tr -d '\r'
+            } -- %reg{g} %reg{s} %arg{@}
+            set-option buffer filetype grep
+            set-option buffer jump_current_line 0
+        }
     }
 }
 complete-command grep file 
diff --git a/rc/tools/make.kak b/rc/tools/make.kak
index 4e148547a..65db8d8ca 100644
--- a/rc/tools/make.kak
+++ b/rc/tools/make.kak
@@ -12,11 +12,13 @@ define-command -params .. -docstring %{
     make [<arguments>]: make utility wrapper
     All the optional arguments are forwarded to the make utility
 } make %{
-    evaluate-commands -try-client %opt{toolsclient} %exp{
-        fifo -scroll -name *make* -script %%{
+    evaluate-commands -try-client %opt{toolsclient} %{
+        fifo -scroll -name *make* -script %{
             trap - INT QUIT
-            %opt{makecmd} "$@"
-        } -- %%arg{@}
+            makecmd=$1
+            shift
+            eval "$makecmd \"\$@\""
+        } -- %opt{makecmd} %arg{@}
         set-option buffer filetype make
         set-option buffer jump_current_line 0
     }
-- 
2.47.1.284.g6ea2d9d271