Seirdy / term-dmenu

Replace dmenu with a floating terminal and FZF! Comes with helper scripts for program/app launching, window switching, etc.
https://gitlab.com/Seirdy/term-dmenu
Other
49 stars 1 forks source link

term-dmenu breaks with large numbers of executables #1

Closed sbuller closed 2 years ago

sbuller commented 4 years ago

Linux seems to have a 128KiB limit on environment variables. It causes a problem in the term-dmenu script on line 21 - export input. Any command after that seems to fail.

I've dealt with the issue in my case with the following change:

diff --git a/dmenu-runner b/dmenu-runner
index d0f0aa1..bc62863 100755
--- a/dmenu-runner
+++ b/dmenu-runner
@@ -25,7 +25,7 @@ fi
 # This may include the occasional non-executable file

 # shellcheck disable=SC2046 # word-splitting isn't an issue here.
-command_list=$(stest -flx $(echo "$PATH" | tr : ' ' | sort -u))
+command_list=$(stest -flx $(echo "$PATH" | tr : '\n' | sort -u) | sort -u)

 # read existing command history

This style of launcher recently occurred to me, and I was very happy to find someone had already done the hard work. Thanks for sharing.

jcgruenhage commented 2 years ago

I've also run into this, but instead of working around it, I wanted to just pass the list in without using env vars. I tried making it work with the named pipe already present, but couldn't get it to work. Instead of having both files and named pipes, I decided to just only use files, which brought me to this patch in the end:

diff --git a/term-dmenu b/term-dmenu
index b64b12b..9dc7039 100755
--- a/term-dmenu
+++ b/term-dmenu
@@ -5,21 +5,21 @@
 # Requires a POSIX-compliant shell, fzf, and abduco.

 export FZF_DEFAULT_OPTS="$* $FZF_DEFAULT_OPTS"
+export TERM_DMENU_DIR="/tmp/term-dmenu-$$"

 # This happens in three steps:

 cleanup() {
-   rm -f /tmp/term-dmenu
+   rm -rf ${TERM_DMENU_DIR}
 }
 trap cleanup EXIT
-# 1. if a named pipe for term-dmenu doesn't exist, create it
-[ -p /tmp/term-dmenu ] || mkfifo /tmp/term-dmenu &
+# 1. crate temporary directory
+mkdir ${TERM_DMENU_DIR}

-# 2. export stdin, separated by newlines, so the terminal process can
-#    access it
+# 2. write stdin to a temporary file, separated by newlines, so the terminal
+#    process can access it
 IFS=$(printf '\n')
-input=$(cat)
-export input
+cat - >${TERM_DMENU_DIR}/input

 # 3. open a floating terminal with supressed stderr, running a shell
 #    command that does the following:.
@@ -29,10 +29,10 @@ export input
 # session needs to stay alive after the terminal closes
 # shellcheck disable=SC2016 # I don't want expressions to expand
 floating-terminal dash -c \
-   'output=$(echo "$input" | fzf); export output; abduco -rnf term-dmenu dash -c "echo \"$output\" >/tmp/term-dmenu" 2>/dev/null' 2>/dev/null
+   'output=$(fzf < ${TERM_DMENU_DIR}/input); export output; abduco -rnf term-dmenu dash -c "echo \"$output\" >${TERM_DMENU_DIR}/output" 2>/dev/null' 2>/dev/null

-# 4. send the value from the named pipe to stdout
-cat </tmp/term-dmenu
+# 4. send the value to stdout
+cat <${TERM_DMENU_DIR}/output

 # vi:ft=sh

I encountered this when trying to use term-dmenu with passmenu, which just hang before. Together with this patch, it works just fine.

Seirdy commented 2 years ago

Fixed in 9269766b9f37defc7deb70a23bc59944aaa65405

jcgruenhage commented 2 years ago

@Seirdy term-dmenu still doesn't work with input lists larger than 128KiB though. For use with passmenu, I'm currently at 260KiB of entries, so I think a patch that doesn't rely on env vars should still be merged.

Seirdy commented 2 years ago

On Sat, Mar 19, 2022 at 02:24:10AM -0700, Jan Christian Grünhage wrote: @.*** term-dmenu still doesn't work with input lists larger than 128KiB though. For use with passmenu, I'm currently at 260KiB of entries, so I think a patch that doesn't rely on env vars should still be merged.

Oops; will get on this later today.

-- /Seirdy

jcgruenhage commented 2 years ago

ftr, such a patch (although using temporary files instead of named pipes) would be https://github.com/jcgruenhage/term-dmenu/commit/71c0632b389214466e2fc703fa2d16c7940795a5, which is what I'm currently using for passmenu.

The patch also has the benefit of including the PID in the temporary files, which means that multiple invocations running at the same time are not potentially racy anymore.