First cut cmdliner support

[rgrinberg]

Attempted to complete one of the TODO items listed in the repository.

The only intended change to the command line itself is just add an explicit
subcommand for building targets. I'd recommend to keep that because default
subcommands are unnatural and somewhat rare.

Feel free to change/modify anything according to your taste otherwise.

TODO:

• Get rid of that hack to get Cmdliner building without result. Or add result
• Add proper manual pages/help

[ghost]

I think we don't need this file

[ghost]

Note that the current default is 4 for -j

[ghost]

Thanks, that looks good. However, when I run jbuilder build --help, I can see that options are named ---j, ---drules instead of -j or --drules.

Regarding the default subcommand, I see your point but I think that's what most build systems do though and I would expect jbuilder <target> to just work. I think it's good to have an explicit build subcommand but it should still be the default.

I have two other questions, as I don't know cmdliner well:

• I see that the internal function still does a manual pattern matching, is it possible to define subsubcommands with cmdliner?
• Do you know if the common options could be grouped under a section COMMON OPTIONS in the man pages?

Regarding the result hack, I'm fine with just adding the Result module, the less modifications we do the vendored code, the easier it will be to upgrade it

[rgrinberg]

On 02/16, Jérémie Dimino wrote: Thanks, that looks good. However, when I run `jbuilder build --help`, I can see that options are named `---j`, `---drules` instead of `-j` or `--drules`.

I fixed this. But the long rule names are changed to long option style --drules. This is the convention that cmdliner enforces, and it's pretty standard so I think it's a positive change.

Regarding the default subcommand, I see your point but I think that's what most build systems do though and I would expect `jbuilder <target>` to just work. I think it's good to have an explicit `build` subcommand but it should still be the default.

OK I changed the default subcommand. I still think that it's not so good usability wise. We could easily have a subcommand and a target named the same way. And most other builds don't support subcommands anyway, so the comparison is a bit artificial.

I have two other questions, as I don't know cmdliner well: - I see that the `internal` function still does a manual pattern matching, is it possible to define subsubcommands with cmdliner?

It's possible but I also don't know how to do it. I'm sure it can be done as opam clearly does it, but I do think that it requires writing more code than you'd expect. I'll look at this more in depth but I think this can be postponed tbh.

- Do you know if the common options could be grouped under a section `COMMON OPTIONS` in the man pages?

I tried fixing this, what do you think?

Regarding the result hack, I'm fine with just adding the `Result` module, the less modifications we do the vendored code, the easier it will be to upgrade it

OK. But how do I do this though? It doesn't seem like bootstrapping is using findlib which is the only way I know of adding result. Also, it's a bit of a shame we have to add the dependency on findlib.

[ghost]

I fixed this. But the long rule names are changed to long option style --drules.
This is the convention that cmdliner enforces, and it's pretty standard so I
think it's a positive change.

Agreed

OK I changed the default subcommand. I still think that it's not so good
usability wise. We could easily have a subcommand and a target named the same
way. And most other builds don't support subcommands anyway, so the comparison
is a bit artificial.

That's true. But you can always do jbuilder build <target> in case of conflict and build is the most used command so IMO it's better to shorten it

It's possible but I also don't know how to do it. I'm sure it can be done as
opam clearly does it, but I do think that it requires writing more code than
you'd expect. I'll look at this more in depth but I think this can be postponed
tbh.

Ok. I agree that's this can be postponed

I tried fixing this, what do you think?

Thanks, the doc looks better now

OK. But how do I do this though? It doesn't seem like bootstrapping is using
findlib which is the only way I know of adding result. Also, it's a bit of a
shame we have to add the dependency on findlib.

We can just add a Result module in jbuilder, we don't need to use the actual result package. If we want to support 4.02 where Pervasives.result is not available, we can simply define a new type. Although I haven't tested 4.02 compatibility

[ghost]

I moved cmdliner sources to vendor/cmdliner and moved the CLI code to bin/main.ml to speed up the bootstrap. This should be out soon.

BTW I found this issue about supporting nested commands: dbuenzli/cmdliner#24

[ghost]

In the end setting build as the default doesn't seem to work well when the first argument is a target. I restored the default to printing the man page