Completion improvements for 111

[ALSchwalm]

Fixes #111

This resolves the issue with trailing '/' missing for directories, and adds completion support for the 'local-bin' case (That is, when a user is typing a path as the first item, only directories and executable files are listed).

I did encounter that double '/' issue you mentioned. It does appear to originate from glob itself, but only happens with '/' and './'. I have no idea why. I've chosen to special case them in expand. Alternately, we could drop the GLOB_MARK and append the directory '/' ourselves.

[andrewchambers]

Hmm, this failure is annoying, need to think about how things should work.

[andrewchambers]

@ALSchwalm I am wondering if we can just add the "/" ourselves for now in completions, hmm. The problem is the reuse of . In bash if I go: ```ls /``` I don't get trailing slashes, so the failing test seems correct.

[andrewchambers]

@ALSchwalm an FYI, these two issues will be very useful for janet shell completions in the future:

https://github.com/janet-lang/janet/issues/121 https://github.com/janet-lang/janet/issues/120

[ALSchwalm]

Ah yeah, those will definitely be handy. Regarding your other note, I can implement the '/' to avoid that special casing (something like below):

(->> (glob (string ;(peg/match expand-parser s)))
       (map (fn [f] 
              (if-let [stat (os/stat f)]
                (if (and (= (stat :mode) :directory)
                         (not= (last f) ("/" 0)))
                  (string f "/")
                  f)
                f))))

However, I also wanted to note that as of bash 5.0.7, I do get the trailing slash:

[adam@laptop /]$ ls /*
bin/   dev/   home/  lib64/ mnt/   proc/  run/   srv/   tmp/   var/   
boot/  etc/   lib/   media/ opt/   root/  sbin/  sys/   usr/

So I'm wondering if I'm misunderstanding your concern.

[andrewchambers]

Oh ok, for me this does not:

[ac@black:~]$ echo /*
/bin /boot /dev /etc /home /mnt /nix /opt /proc /root /run /secrets /sys /tmp /usr /var

My concern is adding GLOB_MARK changes what this does with the current implementation:

echo $PWD

Unless I have misunderstood something about the failing test case. This is because $PWD calls sh/expand, which internally uses glob.

[andrewchambers]

To clarify a bit more, I think for now adding a trailing slash inside the completion function is best (not in expand). I don't mind implementing that change, I know I may be being picky, but i think a small difference like this could be annoying in the future.

[andrewchambers]

If you are out of time, I'm also happy to make the final tweaks and then merge.

[ALSchwalm]

I have some time to implement this. I was misreading the test failures, I see what you're saying now. I think I've also found another edge case that I want to fix (ls /*)

[ALSchwalm]

I think that should do it. I decided to ignore the ls /* case for the time being.

Some refactoring could probably reduce the number of stat calls though.

[andrewchambers]

Cool, I will test it now, I agree about stat syscalls, I can make a refactor ticket.