optimal expansions?

optimal expansions?

[Ray Andrews]

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

Script:

     grn=$'\e[32;1m'
     nrm=$'\e[0m'

     var=( "${(@f)$(apt-file search $1)}" )
     targ=
     var2=()

     for ((i=1; i<=$#var; i++ )); do
         if [[ "$targ" != "${${=var[i]}[1]}" ]]; then
             targ="${${=var[i]}[1]}"
             var2+="\n${grn}${${=var[i]}[1]}${nrm}" # Copy first word of 
line.
         fi
         var2+="${${=var[i]}[2,-1]}" # Copy the rest of the line no 
matter how many words.
     done

     print -l "$var2[@]"

---------------------------------------------------

That's my preferred way to look at 'apt-file search' output (Debian and 
derivatives only of course).  It works fine and I think I understand all 
the expansions and splitting.  One you get used to it the nested 
expansions aren't so scary, just read them from inside out, one step at 
a time and it's easy.  But is it optimal?  I've been known to go one 
step to the right by first going three steps to the left, then four 
steps to the right.  I'm guessing it's tight, but ...

BTW, I had a much simpler way of doing this based on a 'two words per 
line of output' assumption, but Debian, in their wisdom, have a very few 
installable files that have spaces in their names, so the above: "   
[2,-1]   " way of doing things is needed.  I have to split on lines and 
then sub-split on words ... I think.  But I do have a talent for making 
things harder than they need be.


[-- Attachment #2: Type: text/html, Size: 1993 bytes --]

Re: optimal expansions?

[Lawrence Velázquez]

On Fri, Apr 19, 2024, at 3:22 PM, Ray Andrews wrote:
> That's my preferred way to look at 'apt-file search' output (Debian and 
> derivatives only of course).  It works fine and I think I understand 
> all the expansions and splitting.  One you get used to it the nested 
> expansions aren't so scary, just read them from inside out, one step at 
> a time and it's easy.  But is it optimal?

How is anyone supposed to answer this question, when you haven't
deigned to mention what your code is supposed to DO?  You seem to
colorize certain portions of the output by bracketing them with
escape sequences, but which portions, exactly?  What does
"apt-file search" typically output?

Without knowing more about the bigger picture, the best we can do
is nitpick your code without addressing its larger structural issues.
(Unless someone wants to guess about your intentions.)


> Script:
>
>     grn=$'\e[32;1m'
>     nrm=$'\e[0m'
>
>     var=( "${(@f)$(apt-file search $1)}" )

You don't check the exit status of "apt-file", so if it happens to
fail for any reason, your code plows on obliviously, printing nothing
and exiting with a zero status.  You should do something like

	var=( "${(@f)$(apt-file search -- $1)}" ) || return

(assuming this is all in a function, since you have yet again chosen
to not provide any context)


>     targ=
>     var2=()
>        
>     for ((i=1; i<=$#var; i++ )); do

Since you only ever use "i" in the expansion "${=var[i]}", there
is no reason to use this form of "for".  You can just use the usual

	for x in "$var[@]"; do

and subsequently "$x" instead of "$var[i]".


>         if [[ "$targ" != "${${=var[i]}[1]}" ]]; then
>             targ="${${=var[i]}[1]}"
>             var2+="\n${grn}${${=var[i]}[1]}${nrm}" # Copy first word of 
> line.

You're doing that thing again, where you use a literal backslash-n
and rely on "print" to interpret it as a newline.  This is bad
practice here because the rest of the string is the arbitrary output
of an external command, which you do not control.  It could easily
contain substrings that are meaningful to "print".

To insert an empty line in your output, just add an empty element
to "var2" in the desired position.

	% var=(a)
	% var+=
	% var+=b
	% typeset -p var
	typeset -a var=( a '' b )
	% print -rC1 -- "$var[@]"
	a

	b


>         fi
>         var2+="${${=var[i]}[2,-1]}" # Copy the rest of the line no 
> matter how many words.

You should store the result of "${=var[i]}" in a temporary variable
and use that, instead of repeatedly word-splitting the same string
over and over and over.


>     done
>        
>     print -l "$var2[@]"

Use "print -r" to prevent "print" from interpreting escape sequences
in its arguments.

As I mentioned in a previous email, if "var2" has no elements,
"print -l" will print one empty line anyway.  Use "print -C1"
instead.

	print -rC1 -- "$var2[@]"


-- 
vq

Re: optimal expansions?

[Ray Andrews]

[-- Attachment #1: Type: text/plain, Size: 2936 bytes --]



On 2024-04-19 13:40, Lawrence Velázquez wrote:
> On Fri, Apr 19, 2024, at 3:22 PM, Ray Andrews wrote:
>> That's my preferred way to look at 'apt-file search' output (Debian and
>> derivatives only of course).  It works fine and I think I understand
>> all the expansions and splitting.  One you get used to it the nested
>> expansions aren't so scary, just read them from inside out, one step at
>> a time and it's easy.  But is it optimal?
> How is anyone supposed to answer this question, when you haven't
> deigned to mention what your code is supposed to DO?
I thought I was crystal clear what it is supposed to do: reformat 
'apt-file search' output.  The code runs, if you have a Debian 
derivative fire it up.

> You seem to
> colorize certain portions of the output by bracketing them with
> escape sequences, but which portions, exactly?  What does
> "apt-file search" typically output?
I wasn't expecting anyone but Debian users to comment.  Unless there are 
issues that are apparent just from scanning the code.

> You don't check the exit status of "apt-file", so if it happens to
> fail for any reason
It's taken care of, just not within that minimal code I showed.

> ((i=1; i<=$#var; i++ )); do
> Since you only ever use "i" in the expansion "${=var[i]}", there
> is no reason to use this form of "for".  You can just use the usual
>
> 	for x in "$var[@]"; do
Ah!  Now there's a good idea.  I tend to use the above only on  the 
command line, and the more 'formal' for loop in code -- seems more like C.
> and subsequently "$x" instead of "$var[i]".
Right!  Simpler.
>>          if [[ "$targ" != "${${=var[i]}[1]}" ]]; then
>>              targ="${${=var[i]}[1]}"
>>              var2+="\n${grn}${${=var[i]}[1]}${nrm}" # Copy first word of
>> line.
> You're doing that thing again, where you use a literal backslash-n
> and rely on "print" to interpret it as a newline.  This is bad
> practice here because the rest of the string is the arbitrary output
> of an external command, which you do not control.  It could easily
> contain substrings that are meaningful to "print".
I've come to appreciate the problem!  Now for healthy solutions.
> To insert an empty line in your output, just add an empty element
> to "var2" in the desired position.
>
> 	% var=(a)
> 	% var+=
> 	% var+=b
> 	% typeset -p var
> 	typeset -a var=( a '' b )
> 	% print -rC1 -- "$var[@]"
Very good, I'll implement that.

> You should store the result of "${=var[i]}" in a temporary variable
> and use that, instead of repeatedly word-splitting the same string
> over and over and over.
Right.  Three uses, so a temporary var would earn it's keep.
>>      print -l "$var2[@]"
> Use "print -r" to prevent "print" from interpreting escape sequences
> in its arguments.
... and that meshes with getting rid of the '\n's, yes?

Thanks.  It's the difference between something that merely works, with 
something that's genuinely well coded.

[-- Attachment #2: Type: text/html, Size: 5282 bytes --]

Re: optimal expansions?

[Roman Perepelitsa]

On Fri, Apr 19, 2024 at 11:22 PM Ray Andrews <rayandrews@eastlink.ca> wrote:
>
> Script:
>
>     grn=$'\e[32;1m'
>     nrm=$'\e[0m'
>
>     var=( "${(@f)$(apt-file search $1)}" )
>     targ=
>     var2=()
>
>     for ((i=1; i<=$#var; i++ )); do
>         if [[ "$targ" != "${${=var[i]}[1]}" ]]; then
>             targ="${${=var[i]}[1]}"
>             var2+="\n${grn}${${=var[i]}[1]}${nrm}" # Copy first word of line.
>         fi
>         var2+="${${=var[i]}[2,-1]}" # Copy the rest of the line no matter how many words.
>     done
>
>     print -l "$var2[@]"

I'd do it like this:

    #!/usr/bin/env zsh

    emulate zsh -o no_unset -o pipe_fail -o err_exit

    () {
      local last_pkg pkg file
      apt-file search "$1" | while read -r pkg file; do
        if [[ $pkg != $last_pkg ]]; then
          [[ -n $last_pkg ]] && print
          last_pkg=$pkg
          print -P "%B%2F${pkg//\%/%%}%f%b"
        fi
        print -r -- $file
      done
    } "$@"

This implementation fixes a couple of bugs in addition to those
mentioned by Lawrence: it gets rid of the empty line at start of the
output, and leaves whitespace in file names unchanged.

Roman.

Re: optimal expansions?

[Ray Andrews]

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]



On 2024-04-20 00:42, Roman Perepelitsa wrote:

Hafta start using lines like this all the time.  You guys never start a 
function without one of these 'emulate' instructions -- make sure that 
the func has the environment it needs.
> emulate zsh -o no_unset -o pipe_fail -o err_exit
>      () {
>        local last_pkg pkg file
So you break it into words automatically here, and any spaced filenames 
end up in 'file' anyway!
>        apt-file search "$1" | while read -r pkg file; do
>          if [[ $pkg != $last_pkg ]]; then
>            [[ -n $last_pkg ]] && print
>            last_pkg=$pkg
... this line is Basque to me.  It works but I have no idea how. Anyway 
that's for me to research.  It baffles me that there's color codes in there.
>            print -P "%B%2F${pkg//\%/%%}%f%b"
>          fi
Seems we want '-r' most of the time.  And the '--' should always be 
there.  Now that I know not to use newlines for splitting.  And of 
course that obviates most of my '(@f)'s too.
>          print -r -- $file
>        done
>      } "$@"
>
> This implementation fixes a couple of bugs in addition to those
> mentioned by Lawrence: it gets rid of the empty line at start of the
> output, and leaves whitespace in file names unchanged.
To pick nits I want the leading empty line.  But I'm betting your code 
will be faster.  I'll keep Lawrence's edits until such times as I 
understand that 'print -P' tho.



[-- Attachment #2: Type: text/html, Size: 2455 bytes --]

Re: optimal expansions?

[Lawrence Velázquez]

On Sat, Apr 20, 2024, at 10:23 AM, Ray Andrews wrote:
> ... this line is Basque to me.  It works but I have no idea how.  
> Anyway that's for me to research.  It baffles me that there's color 
> codes in there.
>>           print -P "%B%2F${pkg//\%/%%}%f%b"
>>         fi

The "-P" option tells "print" to interpret prompt escapes like "%B"
and "%F" (see zshmisc(1)), allowing you to avoid hardcoding ECMA-48
escape sequences.  The "${pkg//\%/%%}" expansion interpolates the
package name with all "%" characters escaped.


> Seems we want '-r' most of the time.

Yes, unless you actually do want "print" to interpret substrings
like "\n".  Sometimes you do, sometimes you don't.  If the operands
are dynamic and not under your control, you generally don't.


> Now that I know not to use newlines for splitting.  And of 
> course that obviates most of my '(@f)'s too.

That's because Roman's solution uses "read" to stream the output
from "apt-file" instead of saving it all and postprocessing it.
This doesn't have anything to do with "print" though.


> But I'm betting your code will be faster.

If speed is a concern...

% cat af_awk
#!/bin/sh -

apt-file search -- "$1" | awk '
	BEGIN {
		fg_bld_grn = "\33[1;32m"
		reset = "\33[0m"
	}

	$1 != last_pkg {
		last_pkg = $1
		printf "\n%s%s%s\n", fg_bld_grn, $1, reset
	}

	{
		# Preserve runs of blanks in paths.
		print substr($0, 1 + index($0, " "))
	}

	END {
		# Lack of input implies that apt-file failed.
		exit !NR
	}
'
% apt-file search .txt | wc -l
138962
% for x in ray roman awk; do eval time ./af_$x .txt; done >/dev/null
./af_ray .txt  39.43s user 1.74s system 102% cpu 40.099 total
./af_roman .txt  12.99s user 24.97s system 130% cpu 29.069 total
./af_awk .txt  3.08s user 1.02s system 131% cpu 3.131 total


-- 
vq

Re: optimal expansions?

[Ray Andrews]

[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]



On 2024-04-20 15:54, Lawrence Velázquez wrote:
> The "-P" option tells "print" to interpret prompt escapes like "%B"
> and "%F"
Right, I remember that now, the '%' escapes.  So there's an advantage to 
using that method I take it. Never used it anywhere but in my prompt.  
So those escapes are part of the wider world then.
>   (see zshmisc(1)), allowing you to avoid hardcoding ECMA-48
> escape sequences.  The "${pkg//\%/%%}" expansion interpolates the
> package name with all "%" characters escaped.
Ok, I can parse that now.
>> Seems we want '-r' most of the time.
> Yes, unless you actually do want "print" to interpret substrings
> like "\n".  Sometimes you do, sometimes you don't.  If the operands
> are dynamic and not under your control, you generally don't.
Yup, with me using newlines to split arrays, I had to avoid it. It's 
almost amusing, I had the newlines thing exactly backwards but still 
managed to make my functions work.
> That's because Roman's solution uses "read" to stream the output
> from "apt-file" instead of saving it all and postprocessing it.
Right, it has to be more efficient.
> If speed is a concern...
Not in practice, it was just a comment.
...
> % apt-file search .txt | wc -l
> 138962
> % for x in ray roman awk; do eval time ./af_$x .txt; done >/dev/null
> ./af_ray .txt  39.43s user 1.74s system 102% cpu 40.099 total
> ./af_roman .txt  12.99s user 24.97s system 130% cpu 29.069 total
> ./af_awk .txt  3.08s user 1.02s system 131% cpu 3.131 total
That's astonishing.  awk that much faster.  And the code is more complex 
to look at.  I'd have expected zsh native code to always be faster than 
calling an external program.  Thereyago.

>

[-- Attachment #2: Type: text/html, Size: 3370 bytes --]

Re: optimal expansions?

[Roman Perepelitsa]

On Sat, Apr 20, 2024 at 6:24 PM Ray Andrews <rayandrews@eastlink.ca> wrote:
>
> On 2024-04-20 00:42, Roman Perepelitsa wrote:
>
> >
> >           print -P "%B%2F${pkg//\%/%%}%f%b"
>
> Seems we want '-r' most of the time.  And the '--' should always be there.

The missing `-r` is a bug. It should be there. The `--` in this case
is optional because the positional argument cannot start with a dash.

> To pick nits I want the leading empty line.

It's unusual for a script to print a leading empty line. It's best to
avoid doing unusual and surprising things unless you have a good
reason. In this case I would suggest not printing the leading empty
line in the script. If you need it, print it prior to the invocation:

    print && script

Roman.

Re: optimal expansions?

[Ray Andrews]

[-- Attachment #1: Type: text/plain, Size: 807 bytes --]



On 2024-04-21 05:23, Roman Perepelitsa wrote:
>> Seems we want '-r' most of the time.  And the '--' should always be there.
> The missing `-r` is a bug. It should be there. The `--` in this case
> is optional because the positional argument cannot start with a dash.
But the '--' is good standard practice, yes?  It's like putting on your 
turn signal even when there's nobody behind you.  As it was explained to 
me, it's easier to do it than to think about whether you need to do it 
or not.  Just make it a habit.  Meanwhile I have 313K of code that's all 
built around my understanding that 'split on newlines' means add 
newlines where you want to split :-(  ... so I'll slowly go thru it all 
and get that sorted out -- which will remove a hundred '\n's which will 
in turn permit 'print -r'.


[-- Attachment #2: Type: text/html, Size: 1384 bytes --]

Re: optimal expansions?

[Roman Perepelitsa]

On Sun, Apr 21, 2024 at 4:10 PM Ray Andrews <rayandrews@eastlink.ca> wrote:
>
> On 2024-04-21 05:23, Roman Perepelitsa wrote:
>
> Seems we want '-r' most of the time.  And the '--' should always be there.
>
> The missing `-r` is a bug. It should be there. The `--` in this case
> is optional because the positional argument cannot start with a dash.
>
> But the '--' is good standard practice, yes?

You can call it that. It's one of those standard practices that I
don't follow, but if you find the trade-off palatable, go for it.

Roman.

Re: optimal expansions?

[Stephane Chazelas]

2024-04-21 16:19:24 +0200, Roman Perepelitsa:
> On Sun, Apr 21, 2024 at 4:10 PM Ray Andrews <rayandrews@eastlink.ca> wrote:
> >
> > On 2024-04-21 05:23, Roman Perepelitsa wrote:
> >
> > Seems we want '-r' most of the time.  And the '--' should always be there.
> >
> > The missing `-r` is a bug. It should be there. The `--` in this case
> > is optional because the positional argument cannot start with a dash.
> >
> > But the '--' is good standard practice, yes?
> 
> You can call it that. It's one of those standard practices that I
> don't follow, but if you find the trade-off palatable, go for it.
[...]

I would urge you to reconsider, especially for the "print"
built-in of zsh.

print $external_data

or

print -r $external_data

is a command injection vulnerability.

$ external_data='-vpsvar[1$(reboot)]' zsh -c 'print -r $external_data'
System going down for reboot now.

(I have actually singled-out zsh's print for that at
https://unix.stackexchange.com/questions/11376/what-does-double-dash-double-hyphen-mean/590210#590210)

-- 
Stephane

Re: optimal expansions?

[Roman Perepelitsa]

On Sun, Apr 21, 2024 at 6:14 PM Stephane Chazelas <stephane@chazelas.org> wrote:
>
> 2024-04-21 16:19:24 +0200, Roman Perepelitsa:
> > On Sun, Apr 21, 2024 at 4:10 PM Ray Andrews <rayandrews@eastlink.ca> wrote:
> > >
> > > On 2024-04-21 05:23, Roman Perepelitsa wrote:
> > >
> > > Seems we want '-r' most of the time.  And the '--' should always be there.
> > >
> > > The missing `-r` is a bug. It should be there. The `--` in this case
> > > is optional because the positional argument cannot start with a dash.
> > >
> > > But the '--' is good standard practice, yes?
> >
> > You can call it that. It's one of those standard practices that I
> > don't follow, but if you find the trade-off palatable, go for it.
> [...]
>
> I would urge you to reconsider, especially for the "print"
> built-in of zsh.

I'm curious. Which of these would you use?

    print rebooting
    print -r -- rebooting

Or these:

    ls ~
    ls -- ~

I prefer the first form in both of these examples, whether in scripts
or interactively. It's less code, so less time to read and understand.

Roman.

Re: optimal expansions?

[Stephane Chazelas]

2024-04-21 19:39:20 +0200, Roman Perepelitsa:
[...]
> I'm curious. Which of these would you use?
> 
>     print rebooting
>     print -r -- rebooting

Probably print -u2 rebooting, though in scripts, I usually
define functions for reporting like:

info warn err() print -ru2 -C1 -- "$@"
die() { err "$@"; exit 1; }
info rebooting

In any case, for fixed strings, whether to use -- (or - which
also works for print)  or not is not relevant as long as the
string doesn't start with "-" (or "+" for some commands). Same
for -r as longer as arguments don't contain backslashes.

The -- is needed when arguments are not known at code writing
time (or are known to be a problem).

> Or these:
> 
>     ls ~
>     ls -- ~
> 
> I prefer the first form in both of these examples, whether in scripts
> or interactively. It's less code, so less time to read and understand.
[...]

Strictly speaking, it should be ls -- ~ as ~ expands to $HOME
which we can't guarantee won't start with - (though $HOME
starting with - would only happen in pathological cases).

-- 
Stephane

Re: optimal expansions?

[Lawrence Velázquez]

On Sun, Apr 21, 2024, at 4:13 PM, Stephane Chazelas wrote:
> In any case, for fixed strings, whether to use -- (or - which
> also works for print)  or not is not relevant as long as the
> string doesn't start with "-" (or "+" for some commands).

But that was exactly the situation in question -- not quite a fixed
string, but an argument that cannot begin with "-" or "+".

> print -P "%B%2F${pkg//\%/%%}%f%b"

-- 
vq