search

vilicvane / clime

⌨ The command-line interface framework for TypeScript.
252 stars 10 forks source link

Add custom default command name and extension #42

Closed mattyclarkson closed 6 years ago

mattyclarkson commented 6 years ago

This patch allows the customisation of the default command module name and the extension to use for lookups. Depending on the users build they may be using a different extensions for their CommonJS modules. Being able to specify the default command module name allows the user to re-use a nested sub-command as the default command.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.02%) to 97.603% when pulling fe1a2e3bb4ab0f7182f7cc20940b92e171c2723b on mattyclarkson:patch-1 into f594eacd990ebc90d9265db2e4319d505f4abef6 on vilic:master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.007%) to 97.592% when pulling 312c364b64a48a7f8114eec4760af10b6cc0b1e9 on mattyclarkson:patch-1 into 0e075f13cf2b529860fa67d7b06ed1cf091451ff on vilic:master.

vilicvane commented 6 years ago

Hi, thank you for this PR! (And sorry for the late reply.)

Though I have two major concerns:

  1. It doesn't cover help information building.
  2. Maybe we should add an private options: CLIOptions as the third argument of CLI constructor with those two options, and turn related static methods into instance methods?
mattyclarkson commented 6 years ago

  1. I could update the help.ts to use the constants from CLI?

  2. That would make more sense. I initially added exactly that but realised there were static methods that require the same information and didn't want to land a big refactor in a PR if you weren't going to be happy with it. Would you expect that information to be passed to the nested sub-commands?

vilicvane commented 6 years ago

You are right. It seems that this could lead to a major refactoring. Maybe we should just use the static options you added this time and use them directly in helper.ts.

I think I may need a command resolver abstraction in the future.

mattyclarkson commented 6 years ago

@vilic just hit this again. Still good for the above comment changes, to update help.ts to use the static values?

mattyclarkson commented 6 years ago

@vilic Have updated the help.ts as you said. Would love to get this in.

vilicvane commented 6 years ago

@mattyclarkson Thanks! I made some coding style updates and minor tweaks. Will merge after CI success.