Page MenuHomePhabricator

rhg: add a limited `rhg root` subcommand
ClosedPublic

Authored by acezar on Fri, Jun 5, 10:31 AM.

Details

Summary

Clap has been choosen for argument parsing for the following reasons:

  • it's a wildly used and maintained crate
  • it can deal with OS encoding making it suitable for any encoding
  • it supports nonambiguous prefix matching as already available in hg
  • it will soon allow for structopts-style declarative pattern instead of the currently used builder pattern

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

acezar created this revision.Fri, Jun 5, 10:31 AM
Alphare requested changes to this revision.Fri, Jun 5, 11:45 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/rhg/src/main.rs
15

Clap will exit here with the help message and an error code. Use get_matches_safe to properly fall back to "UNIMPLEMENTED_COMMAND" if need be.

This revision now requires changes to proceed.Fri, Jun 5, 11:45 AM

The question of "how are we testing it" is going to appears. At first we can probably have a small test-rhg.t that checks a handful of command. But quickly, we will probably want to integrate this in the main test suite. Having a small piece of logic that recognise call we can fast path and switching them to call to rhg.

acezar updated this revision to Diff 21643.Tue, Jun 16, 12:19 PM
Alphare accepted this revision.Mon, Jun 22, 10:46 AM

nit: "it's a wild used" -> "it's a wildly used" and "it can deal with bytes"... technically it deals with OS encoding. Which is good for our purposes, but is slightly different.

acezar marked an inline comment as done.Wed, Jun 24, 9:04 AM
acezar edited the summary of this revision. (Show Details)
acezar updated this revision to Diff 21698.
Alphare requested changes to this revision.Wed, Jun 24, 10:04 AM
Alphare added inline comments.
rust/rhg/tests/test_t.rs
4 ↗(On Diff #21698)

I didn't see this test in my first pass, but I don't think it should exist. It creates a dependency on Mercurial from pure-Rust code through hardcoded paths and serves very little purpose.

This revision now requires changes to proceed.Wed, Jun 24, 10:04 AM
acezar updated this revision to Diff 21724.Mon, Jun 29, 4:20 AM
Alphare accepted this revision.Wed, Jul 1, 5:15 AM
acezar marked an inline comment as done.Wed, Jul 1, 6:33 PM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.