Re: [hackers] [dmenu][PATCH] Allow only one dmenu_run selection

2023-08-30 Thread Hiltjo Posthuma
On Wed, Aug 30, 2023 at 11:25:18AM -0700, Jeremy wrote:
> On 08/30/23 05:41PM, Christopher Lang wrote:
> > This would be my implementation for executing all selected commands:
> > 
> > #!/bin/sh
> > for x in $(dmenu_path | dmenu "$@"); do
> >   echo "$x" | ${SHELL:-"/bin/sh"} &
> > done
> > 
> > I suppose this behaviour is more intuitive, but it is a little more
> > complicated. What are peoples preferences between original patch and
> > executing all commands?
> > 
> 
> The darker color on the "selected" items makes me think the blocking
> effect of the `$()` makes the most sense, with two exceptions:
>   - the selected options are run in the same order as the input
>   - each selected option is only run once
> 
> Which would look like:
> 
> #!/bin/sh
> t=$(mktemp)
> dmenu_path | tee "$t" | dmenu "$@" | awk 'NR==FNR { x[$0]++; next } x[$0]' - 
> "$t" | sh -x
> 
> Jeremy
> 

If this route is taken I think it would make more sense to not print the lines
directly when CTRL-Enter is used to select multiple lines.
And flush all selected lines on output.

This would make sense for dmenu_run but not some existing scripts that use it.

-- 
Kind regards,
Hiltjo



Re: [hackers] [dmenu][PATCH] Allow only one dmenu_run selection

2023-08-30 Thread Hiltjo Posthuma
On Wed, Aug 30, 2023 at 05:41:28PM +0100, Christopher Lang wrote:
> On Wed, Aug 30, 2023 at 05:48:13PM +0200, Страхиња Радић wrote:
> > On 23/08/30 09:43AM, Randy Palamar wrote:
> > > Personally I see little need for such a patch. I don't really
> > > think anyone is pressing + by mistake; they
> > > are nowhere near each other on most (all?) keyboard layouts.
> 
> A dmenu script in the dmenu repository should set the example for how to
> properly use dmenu. If dmenu_run has this (albiet small) bug, then it
> could make users assume dmenu always outputs a single line.
> 
> > > Actually I didn't even know about this feature despite using
> > > dmenu in a number of different scripts for many years. I'm
> > > already thinking of some use cases.
> 
> If users saw that dmenu_run was handling multi line output from dmenu,
> it could alert them to this feature.
> 
> > I would actually turn this patch the other way around--and have multiple 
> > selection execute all the selected commands.
> 
> This would be my implementation for executing all selected commands:
> 
> #!/bin/sh
> for x in $(dmenu_path | dmenu "$@"); do
>   echo "$x" | ${SHELL:-"/bin/sh"} &
> done
> 
> I suppose this behaviour is more intuitive, but it is a little more
> complicated. What are peoples preferences between original patch and
> executing all commands?
> 
> --
> Christopher
> 

Hi,

If it is a bug then I think it would make more sense to execute all current
output lines.

Currently when you CTRL-Enter on an entry the output will not directly flush.
For example you could type "st" and press CTRL-ENTER 3 times and press escape.
Then it would still open st 3 times:

It is safer to use printf instead of echo, because echo interprets escape
sequences.

Concept patch below:

diff --git a/dmenu.c b/dmenu.c
index 40f93e0..4839f47 100644
--- a/dmenu.c
+++ b/dmenu.c
@@ -490,6 +490,7 @@ insert:
case XK_Return:
case XK_KP_Enter:
puts((sel && !(ev->state & ShiftMask)) ? sel->text : text);
+   fflush(stdout);
if (!(ev->state & ControlMask)) {
cleanup();
exit(0);
diff --git a/dmenu_run b/dmenu_run
index 834ede5..73d77ec 100755
--- a/dmenu_run
+++ b/dmenu_run
@@ -1,2 +1,4 @@
 #!/bin/sh
-dmenu_path | dmenu "$@" | ${SHELL:-"/bin/sh"} &
+dmenu_path | dmenu "$@" | while read -r line; do
+   printf '%s' "${line}" | ${SHELL:-"/bin/sh"} &
+done

-- 
Kind regards,
Hiltjo



Re: [hackers] [dmenu][PATCH] Allow only one dmenu_run selection

2023-08-30 Thread Jeremy
On 08/30/23 05:41PM, Christopher Lang wrote:
> This would be my implementation for executing all selected commands:
> 
> #!/bin/sh
> for x in $(dmenu_path | dmenu "$@"); do
>   echo "$x" | ${SHELL:-"/bin/sh"} &
> done
> 
> I suppose this behaviour is more intuitive, but it is a little more
> complicated. What are peoples preferences between original patch and
> executing all commands?
> 

The darker color on the "selected" items makes me think the blocking
effect of the `$()` makes the most sense, with two exceptions:
- the selected options are run in the same order as the input
- each selected option is only run once

Which would look like:

#!/bin/sh
t=$(mktemp)
dmenu_path | tee "$t" | dmenu "$@" | awk 'NR==FNR { x[$0]++; next } x[$0]' - 
"$t" | sh -x

Jeremy



Re: [hackers] [dmenu][PATCH] Allow only one dmenu_run selection

2023-08-30 Thread Christopher Lang
On Wed, Aug 30, 2023 at 05:48:13PM +0200, Страхиња Радић wrote:
> On 23/08/30 09:43AM, Randy Palamar wrote:
> > Personally I see little need for such a patch. I don't really
> > think anyone is pressing + by mistake; they
> > are nowhere near each other on most (all?) keyboard layouts.

A dmenu script in the dmenu repository should set the example for how to
properly use dmenu. If dmenu_run has this (albiet small) bug, then it
could make users assume dmenu always outputs a single line.

> > Actually I didn't even know about this feature despite using
> > dmenu in a number of different scripts for many years. I'm
> > already thinking of some use cases.

If users saw that dmenu_run was handling multi line output from dmenu,
it could alert them to this feature.

> I would actually turn this patch the other way around--and have multiple 
> selection execute all the selected commands.

This would be my implementation for executing all selected commands:

#!/bin/sh
for x in $(dmenu_path | dmenu "$@"); do
  echo "$x" | ${SHELL:-"/bin/sh"} &
done

I suppose this behaviour is more intuitive, but it is a little more
complicated. What are peoples preferences between original patch and
executing all commands?

--
Christopher



Re: [hackers] [dmenu][PATCH] Allow only one dmenu_run selection

2023-08-30 Thread Страхиња Радић
On 23/08/30 09:43AM, Randy Palamar wrote:
> Personally I see little need for such a patch. I don't really
> think anyone is pressing + by mistake; they
> are nowhere near each other on most (all?) keyboard layouts.
> 
> Actually I didn't even know about this feature despite using
> dmenu in a number of different scripts for many years. I'm
> already thinking of some use cases.

I would actually turn this patch the other way around--and have multiple 
selection execute all the selected commands.


signature.asc
Description: PGP signature


Re: [hackers] [dmenu][PATCH] Allow only one dmenu_run selection

2023-08-30 Thread Randy Palamar
On Wed, Aug 30, 2023 at 6:13 AM Christopher Lang
 wrote:
>
> Using +, multiple selections can be make in dmenu, each
> outputting on a new line. If multiple selections are made in dmenu_run
> then multiple lines will be piped to $SHELL which will not be properly
> handled.
>
> This patch only pipes the last line of the output of dmenu to the shell.
> In effect, the last or only program you selected is run.
>
> I think its quite unintuitive that dmenu can output multiple lines, but
> I understand removing the feature may break some users' scripts. Perhaps
> a command option could be added to dissable multiple output.
> ---
>  dmenu_run | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dmenu_run b/dmenu_run
> index 834ede5..d6bbe5d 100755
> --- a/dmenu_run
> +++ b/dmenu_run
> @@ -1,2 +1,2 @@
>  #!/bin/sh
> -dmenu_path | dmenu "$@" | ${SHELL:-"/bin/sh"} &
> +dmenu_path | dmenu "$@" | tail -n 1 | ${SHELL:-"/bin/sh"} &
> --
> 2.42.0

Hi Christopher,

Personally I see little need for such a patch. I don't really
think anyone is pressing + by mistake; they
are nowhere near each other on most (all?) keyboard layouts.

Actually I didn't even know about this feature despite using
dmenu in a number of different scripts for many years. I'm
already thinking of some use cases.

All the best,
Randy



[hackers] [dmenu][PATCH] Allow only one dmenu_run selection

2023-08-30 Thread Christopher Lang
Using +, multiple selections can be make in dmenu, each
outputting on a new line. If multiple selections are made in dmenu_run
then multiple lines will be piped to $SHELL which will not be properly
handled.

This patch only pipes the last line of the output of dmenu to the shell.
In effect, the last or only program you selected is run.

I think its quite unintuitive that dmenu can output multiple lines, but
I understand removing the feature may break some users' scripts. Perhaps
a command option could be added to dissable multiple output.
---
 dmenu_run | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dmenu_run b/dmenu_run
index 834ede5..d6bbe5d 100755
--- a/dmenu_run
+++ b/dmenu_run
@@ -1,2 +1,2 @@
 #!/bin/sh
-dmenu_path | dmenu "$@" | ${SHELL:-"/bin/sh"} &
+dmenu_path | dmenu "$@" | tail -n 1 | ${SHELL:-"/bin/sh"} &
-- 
2.42.0