Filters should reject empty options

[RicardoM17]

Hello!

First of all, thank you for the great project @DannyBen . It just works and the project seems quite well maintained.

In our project we have a command which has an allowed list with multiple entries. However one of the entries is "" which is also the default value. This hasn't been a issue so far but today I started using this project in MacOS and it failed because of this. For the record I already installed a more up to date version of bash though I'm not exactly sure if that is what is used when running our script.

GNU bash, version 5.2.26(1)-release (aarch64-apple-darwin23.2.0)

The issue seems to be with the following:

if [[ ! $i =~ ^(entry1|entry2|)$ ]]; then

Notice the | after the last entry. The error message is argument must be one of entry1, entry2,

I think the proper fix is to change:

if [[ ! $i =~ ^({{ arg.allowed.join '|' }})$ ]]; then

to

if [[ ! $i =~ ^({{ arg.allowed.reject(&:empty?).join '|' }})$ ]]; then

I wanted to test this but I'm having some difficulties trying to properly compile the bashly project and testing it on my other project. I tried to find some instructions but really couldn't. It's probably not so hard but I never worked with Ruby.

Depending on if the solution above is the correct one I might make a PR to add this fix, though I wouldn't really know if there are other changes that would be required (like tests etc).

Please let me know if there are any other things I should test.

[DannyBen]

Hi, thanks for your kind words and for posting.

Can you provide the most minimal bashly.yml file to demonstrate the issue?

I am trying to understand if this is a bug (most likely), or a misuse/misunderstanding (by that I mean that allowing empty strings ("") are a code smell in my view).

Although I don't mind making the change myself (assuming it is needed), if you want to test it yourself, perhaps these few steps will help:

1. Ensure you have a modern Ruby (>= 3.1) installed
2. Clone the repo and run bundle in it to install dependencies. If the bundle command fails, run gem install bundler.
3. Then you should be able to already run bashly, by running bundle exec bashly (== run it in the context of the bundle)
4. You can make any change you want, this will probably be in one of the *.gtx templates, and rerun.
5. To run tests, you just run rspec
6. There are some other developer goodies in the runfile, which can be accessed by running bundler exec run

But before that, I would prefer seeing a sample YAML so I can reproduce and ponder.

[RicardoM17]

Sure thing, you just need a command with a command with an argument that has the following:

    args: &packages
      - name: package
        help: Which package to use (default=""=all)
        allowed:
          [
            entry1,
            entry2,
            "",
          ]
        default: ""
        repeatable: true

Then in the generated file in bin you will see:

  # :command.whitelist_filter
  eval "input_array=(${args[package]})"
  for i in "${input_array[@]}"; do
    if [[ ! $i =~ ^(entry1|entry2|)$ ]]; then
      printf "%s\n" "package must be one of: entry1, entry2,  " >&2
      exit 1
    fi
  done

We have several people using this in Ubuntu (including myself) and it works, however, whatever version of bash that I'm using on my other machine, so this probably won't break for you but you should still see the | at the end which is probably not nice.

In the meantime in our side I have also made the fix to remove the "" option and use all instead which gets rid of the issue but maybe ideally it would either be rejected, or handled nicely for all machines. Whichever you think is nicest for your project is understandable.

Regarding the steps that you provided they all work ok but for some reason when I'm running it, it wasn't really adding my changes.

After having created a Gemfile with:

gem "bashly", path: "../../bashly"

it now works. My proposed solution does eliminate the issue, but let me know if you'd rather go the route of preventing people from having empty options in those lists.

[DannyBen]

Well - I do not like this part. It is a definite code (configuration) smell.

allowed:
  [
    entry1,
    entry2,
    "",
  ]

Why are you doing it like this?

Try this instead (in a clean, new bashly project to avoid distractions)

# bashly.yml
name: cli
help: Sample application
version: 0.1.0

args:
  - name: package
    help: Which package to use
    allowed:
      - curl
      - httpie
    repeatable: true

# src/root_command.sh
eval "packages=(${args[package]})"

if [ ${#packages[@]} -eq 0 ]; then
  echo "Empty package, use default"
else
  for package in "${packages[@]}"; do
    echo "$package"
  done
fi

What's wrong with that?

[RicardoM17]

The default in this case is "" that's why it was added like that.

That way in.compile.sh we just needed to do build $packages and either a package exists and it is used, or it doesn't and all are compiled.

I've changed it on our side so that default is all and then inside compile.sh I added:

# Replace all with empty string
if [[ $package == "all" ]]; then

    package=""

fi

With your solution we don't seem to have a default option (but maybe it's not required at all?), and at least it doesn't show in the help.

Not saying that it isn't a valid point, just slightly different perhaps.

[DannyBen]

You can specify an array in the default attribute, or even use YAML markers to reference an existing array:

name: cli
help: Sample application
version: 0.1.0

args:
  - name: package
    help: Which package to use
    allowed: &all
      - curl
      - httpie
    repeatable: true
    default: *all

This way, when the arg is not provided, you get the same behavior as when all packages where provided, assuming this is the behavior you are looking for.

👉 Reference

Output:

$ ./cli
curl
httpie

$ ./cli httpie curl
httpie
curl

$ ./cli curl
curl

[RicardoM17]

That is cool to know, but still not quite what I want.

The default for our use case is all which then we clear so that we end up not providing any arguments to the command that we call.

i.e. our cli all -> does an echo "". not an echo "curl hhtpie".

This will then end up compiling all packages but without some issues that can come up with having all packages explicitly set.

That being said from my side, I fixed the issue by replacing "" with all and then replacing it back inside the command.sh as I mentioned above. I am happy with this solution.

I just wanted to raise awareness that as we had it before, it worked without any errors or warnings (which maybe you don't want to have) for some machines, and that doesn't work on another machine/OS which can cause issues to other people in the future.

[DannyBen]

Well, empty string is the same as not provided. My original example demonstrated this exactly. If you haven't received a value, it means it is empty and you can choose to do whatever you want with it.

If you just want to have the wording "Default: All" in your usage, then add it manually:

args:
  - name: package
    help: |-
      Which package to use
      Default: All
    allowed:
      - curl
      - httpie
    repeatable: true