Re: [PATCH] _hdiutil: run candidate-collecting loops outside the pipeline subshell

I apologize for the incorrect diagnosis. My incorrect diagnosis stemmed
from my own usage: I almost always invoke `hdiutil eject` with a mount
point (`hdiutil eject /Volumes/Foo`). Therefore, when `hdiutil eject
/<TAB>` produced no results, I assumed the helper was broken and thought
the issue was with subshell semantics rather than realizing the completion
only offers the bare entity identifier form (`disk4s1`). With an image
actually attached and tabbing from a bare `hdiutil eject `, the existing
helper works as designed.

I'll withdraw this patch and work on another that implements the local/read
-r changes, figure out if we should be using bare `hdiutil` or the full
path (I think bare?) and see if I can add support for offering mount points
and completions.

On Fri, May 29, 2026 at 2:40 PM Mikael Magnusson <[email protected]> wrote:

> On Fri, May 29, 2026 at 7:40 PM Clay Caviness <[email protected]> wrote:
> >
> > The `_hdiutil_disk` and `_hdiutil_device` helpers each use the following
> pattern:
> >
> > _call_program ... hdiutil ... | while read; do
> >   arr+=( ... )
> > done
> > # ... use $arr ...
> >
> > Because every component of a zsh pipeline runs in its own subshell, the
> array assignments  happen in the right-hand subshell and are lost by the
> time the subsequent `_describe` / `_wanted` call runs in the parent shell.
> The practical effect is that `hdiutil eject <TAB>` (which dispatches to
> _hdiutil_disk via the detach|eject branch) never offers any candidates,
> even when disk images are attached. _hdiutil_device has the same shape and
> the same problem for `hdiutil burn -device <TAB>`.
>
> % echo hello | read; echo $REPLY
> hello
>
> > The fix involves feeding the loop via process substitution so the loop
> body runs in the parent shell, allowing the array to persist. I also made
> REPLY local and switched to `read -r` so backslashes in the output are not
> mangled.
>
> I guess local REPLY and read -r changes make sense, but there
> shouldn't be any need to use < <() syntax here. Did you also stop to
> wonder why one of the sites uses a bare "hdiutil" while the other
> hardcodes "/usr/bin/hdiutil"? That seems quite odd to me.
>
> > Patch follows.
> > ---
> > From 2026acc22c87dc98cb985748133f7fee24ebbab5 Mon Sep 17 00:00:00 2001
> > From: Clay Caviness <[email protected]>
> > Date: Fri, 29 May 2026 13:33:39 -0400
> > Subject: [PATCH] _hdiutil: run candidate-collecting loops outside the
> pipeline
> >  subshell
> >
> > Signed-off-by: Clay Caviness <[email protected]>
> > ---
> >  Completion/Darwin/Command/_hdiutil | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/Completion/Darwin/Command/_hdiutil
> b/Completion/Darwin/Command/_hdiutil
> > index 20e69cbc5..ada55bf1d 100644
> > --- a/Completion/Darwin/Command/_hdiutil
> > +++ b/Completion/Darwin/Command/_hdiutil
> > @@ -4,22 +4,24 @@
> >  #
> >  _hdiutil_disk() {
> >    local -a disk_desc
> > -  _call_program devices hdiutil info | while read; do
> > +  local REPLY
> > +  while read -r REPLY; do
> >      local disk_name="${${(M)REPLY[(w)1]%/dev/disk*}#/dev/}"
> >      if (( #disk_name )); then
> >        disk_desc+=( "$disk_name:${${(M)REPLY% *}#?}" )
> >      fi
> > -  done
> > +  done < <(_call_program devices hdiutil info)
> >    _describe -t devices disk disk_desc
> >  }
> >
> >  _hdiutil_device() {
> >    local -a device_desc
> > -  _call_program devices /usr/bin/hdiutil burn -list | while read; do
> > +  local REPLY
> > +  while read -r REPLY; do
> >      if [[ "$REPLY" == [:space:]#IOService:* ]]; then
> >        device_desc+=( "$REPLY" )
> >      fi
> > -  done
> > +  done < <(_call_program devices /usr/bin/hdiutil burn -list)
> >    local expl
> >    _wanted devices expl device compadd "$device_desc[@]"
> >  }
> > --
> > 2.50.1 (Apple Git-155)
> >
>
>
> --
> Mikael Magnusson
>