yonaskolb / Mint

A package manager that installs and runs executable Swift packages
MIT License
2.27k stars 122 forks source link

Required to type command twice when attempting to execute a subcommand #145

Closed bdrelling closed 4 years ago

bdrelling commented 5 years ago

Problem

Whenever I want to run the subcommand of a given executable, I'm required to type the name of the command again.

I'm fairly sure this is because the first one is just referencing the repo and running the default command. I think this is by design, but it feels unintuitive. I actually thought SwiftFormat was broken for a while since mint run swiftformat . kept failing.

System Information

macOS Version: 10.14.5 Shell: zsh Mint Version: 0.12.0

Examples

Examples are using SwiftLint.

mint run swiftlint βœ…

🌱  Using Realm/SwiftLint 0.34.0 from Mintfile.
🌱  SwiftLint 0.34.0 already installed
🌱  Running swiftlint 0.34.0...
Loading configuration from '.swiftlint.yml'
Linting Swift files at paths
Linting 'LinuxMain.swift' (1/57)
<more files>
Done linting! Found 10 violations, 0 serious in 57 files.

mint run swiftlint autocorrect ❌

🌱  Using Realm/SwiftLint 0.34.0 from Mintfile.
🌱  SwiftLint 0.34.0 already installed
🌱  Couldn't find executable "autocorrect"

mint run swiftlint swiftlint autocorrect βœ…

🌱  Using Realm/SwiftLint 0.34.0 from Mintfile.
🌱  SwiftLint 0.34.0 already installed
🌱  Running swiftlint 0.34.0...
Loading configuration from '.swiftlint.yml'
Correcting Swift files at paths
Correcting 'LinuxMain.swift' (1/57)
<more files>
Done correcting 57 files!
alephao commented 4 years ago

I also find this a bit annoying... @yonaskolb is this how it's meant to be? I'm happy to try and fix it if this is a bug :)

bdrelling commented 4 years ago

@yonaskolb any thoughts on this?

yonaskolb commented 4 years ago

Hi all, apologies for the slow response time on this.

I agree that the example you give looks a bit weird

mint run swiftlint swiftlint autocorrect

Why it looks weird is that the first swiftlint actually looks up the repo to run and the second is the actual executable, but it looks confusing as the repo lookup is using a convenience that only requires the name. It wouldn't look as confusing if the examples showed the lookup more verbosely

mint run realm/swiftlint swiftlint autocorrect

The executable being required matches how swift itself works

mint run realm/swiftlint 
swift run #within the Swiftlint repo
mint run realm/swiftlint swiftlint autocorrect
swift run swiftlint autocorrect #within the Swiftlint repo

I agree usage could be cleaner though. There are a couple of options

1.

Don't require the executable unless there are multiple executables

mint run swiftlint autocorrect

Assuming Swiftlint had multiple executables:

mint run swiftlint other-executable command-name

This would throw an error or prompt for the executable if there were multiple and they weren't called autocorrect. If one of the executable ever had a command that shared the same name as one of the other executables this would break. If this were implemented, then for backwards compatibility mint would check if the first argument was the name of one of the executable even if there was only 1 in the package, and then drop that from the arguments.

2.

Same as above but use --executable swiftlint when it is required. This is more flexible and safe but doesn't look as nice Assuming Swiftlint had multiple executables:

mint run swiftlint --executable other-executable command-name

3.

Simply change all the documentation to show the repo in the repo lookup, which makes things less confusing. Not ideal, especially with with a Mintfile as it's designed to be an easy name lookup there.


I'm leaning towards 1, but all the edge cases would need to be tested thoroughly. Thoughts?

guillaumealgis commented 4 years ago

Definitely voting for option 1 πŸ‘

alephao commented 4 years ago

The approach 1 looks more intuitive IMO, but I believe that using the repo name is not as intuitive as looking at the executable name.

Take vapor/toolbox for example. It has an executable called vapor, but the repo is vapor/toolbox. I would use mint run vapor instead of mint run toolbox.

What do you think about the following approach, taken from npx?

When using mint run with a repo argument: mint run <repository> <binary> [args]

  1. If the <repository> has the same name as the binary, you can use mint run realm/swiftlint [args]
  2. If the repo has a different name than the binary, you have to explicitly write the binary name mint run vapor/toolbox vapor [args]

When using mint run without the repo argument: mint run <binary> [args]

  1. It checks if there are any executable matching <binary> on MINT_PATH 1.a. If there is a single executable, use it. 1.b. If there are multiple executables with the same name, then list the repos containing that executable, and the user can pick one.

I believe that backwards compatibility could be achieved by following the same strategy in option 1

for backwards compatibility mint would check if the first argument was the name of one of the executable even if there was only 1 in the package, and then drop that from the arguments.

yonaskolb commented 4 years ago

This has been added in #159. Could you please have a look and test it out?

bdrelling commented 4 years ago

@yonaskolb I'm sorry I never verified way back in February, but just wanted to confirm this was working perfectly! Thought about it a second ago. Thanks for implementing!