Implement CLI changes

[Xophmeister]

The proposed changes to the CLI are wide-reaching. This is the "super-PR", that will ultimately be merged into main, which tracks "sub-PRs" where the functionality is implemented in smaller, more-digestable chunks.

• CLI argument parser (CLI Changes Sub-PR: New structure and parsing logic #591)
  • Global options
  • Subcommands
    • Format¹
    • Visualisation
    • Configuration²
    • Version³
    • Aliases
    • Default subcommand⁴
• Update implementation to use new parsed arguments
  • Format (CLI Changes Sub-PR: Formatting subcommand #599)
  • Visualise (CLI Changes Sub-PR: Visualisation subcommand #597)
  • Configuration
    • Feature parity (and then some) with main (CLI Changes Sub-PR: Configuration Subcommand #593)
    • Implement merge collation (CLI Changes Sub-PR: TOML merge Collation #596)
• Update supplementary tools (CLI Changes Sub-PR: Update supplementary tools #594)
  • TTY playground
  • Pre-commit hook
• Update tests (CLI Changes Sub-PR: Integration tests #601)
• Update documentation (CLI Changes Sub-PR: Documentation #592)
  • README
  • CHANGELOG
  • Migration guide
  • CI⁵

Resolves #566 (accidentally)
Resolves #580
Resolves #588
Resolves #603
Resolves #605

---

Extra toppings:

• Expose the logging level via the command line #607
• Fix language definition cache locking #608

Footnotes

1. The fmt subcommand now accepts directories in its list of FILES..., which it will dutifully expand; this allows you to do, for example, topiary fmt ., rather than relying on shell expansion. ↩

2. Configuration collation is handled slightly differently with this new interface. Previously this was a little confusing/unfamiliar. ↩

3. Creating a version subcommand is straightforward, but it seems redundant to have both that and the preferred --version flag. Also, as version --configuration would make no sense, it would necessitate additional complexity just for the sake of coherent help text. ↩

4. Subcommands can be optional in Clap, but omitting a subcommand -- as though there were a default -- and having Clap interpret that default's arguments is a bit of a hack...and it makes the help text messy. (See Default subcommand clap-rs/clap#975)

   Addendum Default subcommands and arbitrary subcommand abbreviation is considered an anti-pattern, because -- at some point -- someone may rely on this behaviour, which fossilises the interface, preventing future expansion. (See Command Line Interface Guidelines) ↩

5. The CI checks that the help text in the README matches the binary's help text. With subcommands, this is now a little more complicated. ↩

[ErinvanderVeen]

This isn't a review, but I noticed that this PR removes the usage of the builtin query files altogether. I think this is fine, actually, because I'm reworking them at the moment. So I think this can be reintroduced in my PR.

[Xophmeister]

This isn't a review, but I noticed that this PR removes the usage of the builtin query files altogether. I think this is fine, actually, because I'm reworking them at the moment. So I think this can be reintroduced in my PR.

The builtin query files are still used. They're derived from the language, as before:

topiary/topiary-cli/src/io.rs

Lines 140 to 176 in 9d88566

	impl<'cfg, 'i> Inputs<'cfg> {
	pub fn new<T>(config: &'cfg Configuration, inputs: &'i T) -> Self
	where
	&'i T: Into<InputFrom>,
	{
	let inputs = match inputs.into() {
	InputFrom::Stdin(language, query) => {
	vec![(|| {
	let language = language.to_language(config);
	let query = query.unwrap_or(language.query_file()?);
	Ok(InputFile {
	source: InputSource::Stdin,
	language,
	query,
	})
	})()]
	}
	InputFrom::Files(files) => files
	.into_iter()
	.map(|path| {
	let language = Language::detect(&path, config)?;
	let query = language.query_file()?;
	Ok(InputFile {
	source: InputSource::Disk(path, None),
	language,
	query,
	})
	})
	.collect(),
	};
	Self(inputs)
	}
	}

[Xophmeister]

I'm curious if anyone has any insights into this...

It's not clear to me how the following is allowed:

topiary/topiary-cli/src/main.rs

Lines 50 to 64 in 9d88566

	let cache = LanguageDefinitionCache::new();
	let (_, mut results) = async_scoped::TokioScope::scope_and_block(|scope| {
	for input in inputs {
	scope.spawn(async {
	// NOTE "try blocks" and "async closures" are both unstable features. As
	// such, to report errors when they happen -- rather than collated at the
	// end -- we have to resort to this awkward dance, so we can benefit from
	// `?` syntax sugar. Rewrite with a "try block" once the feature is stable.
	let result: CLIResult<()> = match input {
	Ok(input) => {
	let lang_def = match cache.fetch(&input).await {
	Ok(lang_def) => lang_def,
	Err(error) => return Err(error),
	};

Ownership of cache, here, ought to be transferred to each async block that is spawned to do the formatting for that input file. However, you cannot have multiple owners, so this should raise a compilation error. Yet, it doesn't: it both compiles and works correctly.

The usual way around this is to wrap your shared state in an Arc, clone a reference and move that into each future. Something like this:

let cache = Arc::new(LanguageDefinitionCache::new());

let (_, mut results) = async_scoped::TokioScope::scope_and_block(|scope| { 
    for input in inputs {
        scope.spawn({
            let cache = Arc::clone(&cache);

             async move {
                 // ...

I don't know how the code that is checked in gets around this. I suspect async_scoped is doing some dark magick...

[ErinvanderVeen]

This isn't a review, but I noticed that this PR removes the usage of the builtin query files altogether. I think this is fine, actually, because I'm reworking them at the moment. So I think this can be reintroduced in my PR.

The builtin query files are still used. They're derived from the language, as before:

topiary/topiary-cli/src/io.rs

Lines 140 to 176 in 9d88566

	impl<'cfg, 'i> Inputs<'cfg> {
	pub fn new<T>(config: &'cfg Configuration, inputs: &'i T) -> Self
	where
	&'i T: Into<InputFrom>,
	{
	let inputs = match inputs.into() {
	InputFrom::Stdin(language, query) => {
	vec![(|| {
	let language = language.to_language(config);
	let query = query.unwrap_or(language.query_file()?);
	Ok(InputFile {
	source: InputSource::Stdin,
	language,
	query,
	})
	})()]
	}
	InputFrom::Files(files) => files
	.into_iter()
	.map(|path| {
	let language = Language::detect(&path, config)?;
	let query = language.query_file()?;
	Ok(InputFile {
	source: InputSource::Disk(path, None),
	language,
	query,
	})
	})
	.collect(),
	};
	Self(inputs)
	}
	}

I'm talking about the bash(), json(), etc, functions that use the include_str! macro.

[Niols]

I can't comment on the Rust of course, but here's my review of what I could indeed read. Let me say that I am very excited about this rework!