Support "--" in taylor cli to disambiguate arguments for taylor versus arguments for the game

[azimux]

What's the Change?

Before I had to do: taylor game.rb localhost (not a big deal)

But now I can do taylor -- localhost to pass localhost through to my game. Which feels more intuitive.

Note, I also did a follow-up opportunistic refactor commit to get rid of the seemingly-unnecessary/confusing "command" parameter to Run. You can review the commits separately if you don't like the refactor or also I'm happy to tweak stuff based on feedback.

I also streamed working on this so I have video of the whole thing, ha. It's 4 hours total though. But if curious to click around in it I can share!

Checklist

• Added tests

• Added documentation
  ^ I suppose I could update the help output to mention this feature. Let me know if I should.

• Updated migrations/0_4_to_0_5.md if it's a breaking change
  ^ This one is likely irrelevant

• Added an entry to changelog.md

Re:

• Run dx/exec bundle exec rake ci

rake ci:testing passes and I added a couple tests for this. I can't get a passing rake ci due to clang-format errors. My clang-format version is 19.1.7.

I also ran rake to build taylor and tested it with ~/gitlocal/ruby/Taylor/dist/linux/debug/taylor ~/gitlocal/ruby/Taylor/cli-tool/cli.rb -- localhost to verify it works.

Running dx/exec bundle exec rake ci like in the template item gave me the following error:

╚dx/exec bundle exec rake ci
[ dx/exec ] Using 1000:1000 since we're on Linux
[ dx/exec ] Docker is installed!
[ dx/exec ] Executing 'bundle exec rake ci' inside of container with service named 'app'
service "app" is not running

Likely I'm missing some basic step to get that thing running first? This isn't related to my change. I assumed that a local rake ci:testing was probably good enough for a change like this but let me know.

[azimux]

Oh I threw this in here because I kept finding myself typing taylor -v and then having to do taylor --version but I probably should have thrown it in that other misc PR

[HellRok]

I did make a decision when I wrote all these commands to not support short flags as I think they're a bit confusing and by not having them I force people to write easy to understand commands. Please remove this short flag

[azimux]

The current codebase/tool communicates and implements the exact opposite decision.

Are you sure you want me to remove this? It will make the help output a lie again and will make --version the inconsistent oddball again.

I can remove it, of course, but seems incorrect unless the intent is to implement that decision for the other 3 options and to fix the help output. I can also say as one using it that I type taylor -v by habit semi-frequently so there's a bit of an ergonomic loss as well.

[HellRok]

oh shoot, you're right. I think I considered not having short flags but didn't end up doing it. I've just been typing out the long flags for so long I forgot the short ones existed 😆

[HellRok]

I'd prefer not to include the refactor in this pull request as I feel it makes it quite hard to read. I also would prefer not to have the refactor. It's a bit confusing but I think it's more legible to me as it is.

The rest of the code looks pretty good to me and I think I'm happy to merge after you action these things.

[HellRok]

Oh, could you also please use the pull request template going forward? It helps me know what actions you've taken to ensure code quality

[azimux]

Oh, could you also please use the pull request template going forward? It helps me know what actions you've taken to ensure code quality

I've gone ahead and put the PR description into the template format if helpful.

[azimux]

I'd prefer not to include the refactor in this pull request as I feel it makes it quite hard to read. I also would prefer not to have the refactor. It's a bit confusing but I think it's more legible to me as it is.

Ah, that kind of surprises me. I think what I'll do is just move that commit to a different PR for re-consideration and I can point out why I think it reduces confusion instead of increasing confusion. I think that can be gleaned by reading the two commits in isolation but maybe easier as a separate PR. Then you can just close that PR if you still feel the same way.

I've converted this to a draft and will do that likely later today. I'll update the PR description once done (and I'll add a changelog entry) and then I'll convert it back from a draft.

I think I'll also move the -v addition to its own PR as well since that one kind of caught me off guard as well. That should simplify/focus things in this PR.

Cheers!

[azimux]

Alrighty I've moved the controversial parts to other PRs (the -v fix and the command arg removal refactor, which is a PR in my fork since I can't make a PR to my branch but in this repo: azimux#1 but I can open the PR to your repo if this is merged [or put in a branch here somewhere].)

Unless you reviewed the previous two commits in isolation you should probably re-review.

I also added an entry to the changelog.