Skip to content

Conversation

@lukeed
Copy link
Owner

@lukeed lukeed commented Jun 23, 2019

Closes #2 🎉

@lukeed lukeed merged commit 00d355b into master Jun 23, 2019
@lukeed lukeed deleted the feat/single branch June 23, 2019 00:48
@tunnckoCore
Copy link

tunnckoCore commented Jun 23, 2019

I think this could be done better. Allow for command aliases, like in Yargs https://github.com/yargs/yargs/blob/master/docs/advanced.md#default-commands

If found $0 in the command aliases, then this command will act as the main bin.

sirv [dir]

# equivalent to
sirv serve [dir]

# equivalent to
sirv foo [dir]

The command definition would be similar to normal command.

prog.command(['serve', '$0', 'foo'], 'serving something')

Seems more elegant and simple instead of isSingle.

@lukeed
Copy link
Owner Author

lukeed commented Jun 23, 2019

Disagree, that looks awful :)

Sorry, already thought about, discussed, and published.
The isSingle is actually optional, but I'd prefer people set it so that they verify what they're doing.

There's nothing sleeker or more concise than this:

sade('sirv [dir]')
  .option('-H, --host', 'Hostname to use', 'localhost')
  .option('-p, --port', 'Port to use', 5000)
  .action((dir, opts) => {
     // ... 
  })

@tunnckoCore
Copy link

tunnckoCore commented Jun 23, 2019

Yes, it's optional but is limiting. Oriented to one specific case.
The yargs way is fixing this specific case and gives a bonus thing - to alias commands, which is definitely a cool feature.

I have tons of reasons not to use Yargs, but Sade. That's why I'm sad about your way of thinking and decisions.

@lukeed
Copy link
Owner Author

lukeed commented Jun 23, 2019

You're missing the point. A single command binary has no commands – you're only use of the binary is the binary name itself. Why would there ever need to be an alias? That makes no sense.

You also have tons of reasons to not use Sade. No one's forcing you. You must realize that all your tickets and comments in this repository have been about why design of Sade sucks, is limiting, and/or done incorrectly. Not very pleasant to try and accommodate you, I must admit.

@tunnckoCore
Copy link

tunnckoCore commented Jun 23, 2019

You're missing the point.

No, I get that :lau Just said that you could used that to introduce one more feature.

You also have tons of reasons to not use Sade.

Nope, I don't. I even thought to tweet about it before a few days about why it's better.

You must realize that all your tickets and comments in this repository have been about why design of Sade sucks, is limiting, and/or done incorrectly.

Definitely no. Never said that it sucks or the design. And if you thinking of opening issues when you face a problem is wrong, that's bad.

It's always better to open an issue and try to discuss, instead of directly sending PR or even worse mirroring/copying the package. That's how open source works for me, and some times I even waited for years until I take the thing in my hands.

Anyway. 🍷 😉

@lukeed
Copy link
Owner Author

lukeed commented Jun 23, 2019

Issues are never a problem, nor are discussions aiming for change! It's how those discussions are carried out & the phrasing of words that makes the difference. If this is a misinterpretation on my end, I'm sorry, but I waited for multiple instances to verify before mentioning anything. I try to give the benefit of the doubt as long as possible.

For the record, this was discussed over the course of a few weeks with devs and teams who use Sade, especially those who requested this feature in the past. The PR may have been opened & merged within 24 hours, but that was only after it had already been accepted. The sirv example above has been sitting on my drive as a candidate for ~3 months along with the other options.

Here's one that's closest to what you proposed:

sade('sirv')
  .command('[dir]', 'Run a file server', { single: true })
  .option('-H, --host', 'Hostname to use', 'localhost')
  .option('-p, --port', 'Port to use', 5000)
  .action(handler)

While it looks and feels fine, the problem is that it's unclear how/if global settings are to be merged into the command scope (or vice versa). For example

sade('sirv')
  .example('public --dev') // here
  .option('-c, --cors', 'Attach CORS headers') // here
  .command('[dir]', 'Run a file server', { single: true })
  .option('-H, --host', 'Hostname to use', 'localhost')
  .option('-p, --port', 'Port to use', 5000)
  .action(handler)

How do the global attributes (pre-command) get reconciled with the command attributes (post-command)? Sure, in this example, it's easy since A) it's small B) there are no option conflicts. But what if there are? It's not as easy to see. And it also leaves the "in what order should these be shown?" question unanswered.

The design of Sade has always been that you set attributes on the program, as a whole, before adding commands with attributes of their own. In a tree analogy, you start with the trunk before growing a branch.

Well, in "single command mode", there are no branches. It's just a program stump 😆 It's the base/global scope of your program, because that's all your program is. There are no branches because there are no avenues or routes (aka, commands) where you can find extra attributes or behaviors. It's just the one – the root-level stump.

Conceptually, this made much more sense to ~80-90% of the people I discussed this with. The "beauty" in the idea is that it was always already there, by design. We're just now highlighting it.

If you're hung up on the isSingle parameter – skip it. As mentioned before, you don't actually need it.
That argument only exists for the case(s) where your program does not take any named arguments. In such cases, the name would not be customized beyond the binary name itself, which means there has to be some other fallback for declaring the "single command mode" intention.

Those examples look like this:

$ bin [options]
# bin --foo --bar --baz

I do not expect this to be a common use case, because it hardly makes use of any Sade features – but it's supported just in case since it's bound to show up at some point.

// Example of single-bin w/o any arguments
//  ~> AKA, the only time `isSingle` needs defining
sade('sirv', true)
  .option('-H, --host', 'Hostname to use', 'localhost')
  .option('-p, --port', 'Port to use', 5000)
  .action(handler);

// Example of single-bin w/ arguments
//  ~> AKA, pretty much every use case
sade('sirv [dir]')
  .option('-H, --host', 'Hostname to use', 'localhost')
  .option('-p, --port', 'Port to use', 5000)
  .action(handler);

Hope that helps shine some light on the "why" – and sorry again if I misinterpreted your tone 🍻

@tunnckoCore
Copy link

tunnckoCore commented Jun 23, 2019

This feature doesn't affect me in any way, so I don't care and I'm not against it. Just discussing the way it could be introduced. Anyway, it already landed so it may not make much sense to discuss it.

and sorry again if I misinterpreted your tone

Sorry my tone if there's such :D I'm never going with any tone. I don't know why everyone is assuming I'm hating or approaching with anger or bad feelings.

It's how those discussions are carried out & the phrasing of words that makes the difference.

Sure, probably. I'm not native English and sometimes I definitely don't like the way I put it. I'm not good at that and had problems in the past because of that.

Here's one that's closest to what you proposed:

I'm not proposing that. And it definitely doesn't look good and with { single: true } option passed to the .command it's not clear what happens. It's looks and feels more magical and messy than just passing an array of aliases for a command, what I've shown. I may open another issue for that feature, but I don't care about it because I don't need it for now anyway.

This feature is about when you want to have the "single command mode" behavior but have more commands - which in reality we can just call command aliases. The first example that comes to my mind is the Parcel CLI. You can parcel index.html or parcel watch index.html.

For the record, this was discussed over the course of a few weeks with devs and teams who use Sade, especially those who requested this feature in the past. The PR may have been opened & merged within 24 hours, but that was only after it had already been accepted.

Sure thing. That's totally acceptable and I understand.

The design of Sade has always been that you set attributes on the program, as a whole, before adding commands with attributes of their own. In a tree analogy, you start with the trunk before growing a branch.

Of course, great. That's why I like Sade instead of yargs, where I should add some "builder" function or object, so to be able to add command specific options. Sade is intuitive.

🥂

@lukeed
Copy link
Owner Author

lukeed commented Jun 23, 2019

Parcel CLI. You can parcel index.html or parcel watch index.html.

This is the default option. In this case, prog.command('watch', '...', { default: true }) would allow you to omit watch from usage. Also, microbundle does this.

You're only allowed one default command per program – I believe commander does this too.

Aside from that, command aliases aren't available nor planned.

IMO, a command should only ever have one name. Otherwise it's very confusing, actually.
Personally, my least favorite CLIs are those where there's 5 ways to do the same 1 action.
(jackie-chan-why.jpg)

@tunnckoCore
Copy link

Oooh, yea yea, probably. I was just reading their cli code to see what is happening there but seems like they are not using this option and are doing another magic.
Anyway.

Yup. I don't like it either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Is it possible to use 'generic' value as the default command?

3 participants