New CLI API

[lencioni]

While we are converting to JS, I figured it is a good opportunity to redesign the CLI API from scratch to take into account holistically all of the features it has grown to include. Given that all plugins will go through this API, I think it is important that we really get it right. If you are open to this idea, I'd like to use this issue to plan out our ideal API before implementing it.

My first thought is that we should have a few top-level commands instead of flags. I think this would make its usage clearer. These would be something along the lines of word, goto, and fix. Invoking it would maybe look something like:

importjs word Foo --stdin-file-path path/to/file

importjs goto Foo --stdin-file-path path/to/file

importjs fix --stdin-file-path path/to/file

I wonder if --stdin-file-path should just be a required argument instead of a flag:

importjs word Foo path/to/file

The other flags that we currently have are --overwrite, --rewrite, --selections, --version, and --help. I think maybe rewrite should be a command instead of a flag as well.

We should probably also adjust the --selections API from taking pairs of words and positions within the selection list to take pairs of words and paths, so that we can avoid a second lookup. Perhaps selections should be a command as well?

importjs selections Foo:path/to/foo,Bar:path/to/bar path/to/file

I'm just spitballing here. Thoughts?

[trotzig]

You're a first class spitballer. The steps you outline sound like good improvements.

I figured it is a good opportunity to redesign the CLI API from scratch

Agreed.

My first thought is that we should have a few top-level commands instead of flags.

That sounds like a good idea.

I wonder if --stdin-file-path should just be a required argument instead of a flag

That should work, we'll just have to remember to support the commandline usecase where you want to run import-js on a file directly (and not pass it in through stdin). We should be able to support both use-cases though.

I wonder if we can combine it with the word command? We can allow multiple words, comma-separated. And if you want to, you can append an import path too. E.g.

importjs word Foo
importjs word Foo,Bar path/to/file
importjs word Foo:path/to/foo,Bar:path/to/bar path/to/file

Unrelated, but are you suggesting a change for the CLI command name from import-js to importjs? Did you suggest that to disambiguate from the old one?

[lencioni]

Unrelated, but are you suggesting a change for the CLI command name from import-js to importjs? Did you suggest that to disambiguate from the old one?

Oh right. Yes, what do you think about changing the command name to importjs? It feels awkward to me to have a hyphenated command.

[trotzig]

Sure! That would make the switch a bit simpler too.

[trotzig]

Also, what do you think about sending back the entire result as json? Then we could use stderr the way computer gods intended (for displaying errors and warnings). I worry a little bit about the overhead of generating the json blob, and parsing that json again in the editor plugin. Maybe we should just go with what we were already doing: stdout is for the new file contents and stderr for messages.

[trotzig]

I'm currently looking at https://github.com/yargs/yargs. The list of features is impressive, though it makes me worried that it's a bit over-engineered. I'm going to try using it to begin with anyway.

[trotzig]

On second thought, I think I'm going to start with https://github.com/tj/commander.js/. It only has one dependency, yargs has 12 (which ends up at 35 when npm installed).

[lencioni]

Also, what do you think about sending back the entire result as json?

I think we should definitely do this. At least in JS, JSON serialization and parsing is fast so we should be covered for the core tool and the Atom plugin. I'm sure that Python is covered as well.

Vim won't have native JSON parsing until Vim 8, but it looks like someone wrote a JSON parser in vim script, if we want to try to ditch the Ruby dependency: https://github.com/vim-scripts/ParseJSON

[lencioni]

It only has one dependency, yargs has 12 (which ends up at 35 when npm installed).

I'm fine with commander if we can use it to create the interface we want, but I don't think 12 dependencies is a problem--particularly when 4 of them belong to yargs itself.

In looking for comparisons, I found people on HN talking about how much they like http://docopt.org/ so maybe that's worth looking at too? Unfortunately, it looks like this is the closest we can get to JS there: https://github.com/docopt/docopt.coffee

Someone posted this as an example of a subcommand heavy CLI written with commander: https://github.com/jdc0589/mite-node/blob/master/lib/cli/index.js#L37

Some also suggested Chalk: https://github.com/chalk/chalk Edit: looks like chalk is used for coloring CLI output so not very helpful here.

[trotzig]

I'll see what we can accomplish with commander. If you feel strongly about trying others, let me know.

I trust that json parsing is fast, but there's a ton of escaping happening. Anyhow, all speculation. I'll try to do a benchmark.

[trotzig]

Okay, after some rough benchmarking I'm not worried at all about the json parse/stringify overhead. So json it is. 👍

[trotzig]

The first version of the CLI tool has been built. Commits starting at https://github.com/trotzig/import-js/commit/987d3dcceb5570575ccc78e86270218f87c494d3 .

[lencioni]

I did a super quick review. Looks pretty good!