Add trailing slash when looking for directories

[kolayne]

Fixes #22 (although, I'm not sure this is the only place that should be updated)

[sio]

I've checked that by default bash behaves as you decribed here and in the issue. Thank you for reporting this.

This PR requires some extra work before it's ready to be merged. Please check failing CI runs on Linux (ignore macOS and FreeBSD for now because they require custom setup)

From a brief look at failed test cases here are the things that need to be fixed:

• file variable leaks into global scope
• Many test cases need to be updated because they were expecting output without a trailing slash. This will require quite a lot of boring work because blanket-appending slashes to every failed test data could hide some errors (e.g. we must not append slash to paths that are not directories)
• Some changed test outputs introduce not only new slash suffix, but also change the prefix. It's hard to tell if this is acceptable without interactive manual testing. My intuition says it's an error, but I'm not sure. E.g. this snippet in CI output:

 - {'usr/share/', 'usr/somefile'}
 + {'somefile', 'share/'}

Looking from a higher level though... I'm not sure I like the first part of the diff. "FIXME" is kinda a red flag right away, but even without that a duplicated compgen call for a single completion plus a throwaway SUBDIRS array do not seem like a neat solution.

        if [[ "_$ACTION" == "_directory" ]]
        then
            OPTION="dirnames"
        else
            OPTION="filenames"
        fi

How about leaving this section largely intact and setting all compgen keys there? (-o dirnames -S / for directories and -o filenames for regular files). We could call compgen just once then.

[kolayne]

• Many test cases need to be updated because they were expecting output without a trailing slash. This will require quite a lot of boring work because blanket-appending slashes to every failed test data could hide some errors (e.g. we must not append slash to paths that are not directories)

:+1:

• Some changed test outputs introduce not only new slash suffix, but also change the prefix. It's hard to tell if this is acceptable without interactive manual testing. My intuition says it's an error, but I'm not sure.

I have noticed that too, but I failed to figure out when this case appears. Even while manually testing the master version of it, I haven't run into a situation where the prefix of the completion was omitted...

"FIXME" is kinda a red flag right away,

Omg, sorry, I forgot to fix that before opening a PR

a duplicated compgen call for a single completion plus a throwaway SUBDIRS array do not seem like a neat solution.

How about leaving this section largely intact and setting all compgen keys there? (-o dirnames -S / for directories and -o filenames for regular files). We could call compgen just once then.

This does sound better than the current solution. However, it doesn't seem to work on my machine:

nikolay@KoLin:~$ compgen -o dirnames -S / D
Desktop
Docs
Downloads
nikolay@KoLin:~$ 

[sio]

This does sound better than the current solution. However, it doesn't seem to work on my machine: $ compgen -o dirnames -S / D

That's weird. Doesn't work for me too.

Anyways, the idea was to provide all arguments in one variable and then call compgen $ARGS "$INPUT" once. The fact that ARGS will have to be -S/ -d instead of -S/ -o dirnames is an implementation detail 🙂

[kolayne]

Oops, that's accidental.

I don't think I'll ever get back to this, though. Anyone is welcome to use the code as the base for a better implementation, if needed.