CLI names command can search for multiple names

[xmbhasin]

Overview

This change allows the names CLI command to take multiple name/hash arguments to be looked up in the codebase for matching terms.

After this change, the output of names given multiple arguments looks like the following:

scratch/main> names max +

  max:
    Term
    Hash:   ##Float.max
    Names:  builtin.Float.max


  +:
    Terms
    Hash:   ##Float.+
    Names:  builtin.Float.+

    Hash:   ##Int.+
    Names:  builtin.Int.+

    Hash:   ##Nat.+
    Names:  builtin.Nat.+

Closes #3557.

Implementation notes

In Unison.Codebase.Editor.Input, changes Input constructor NamesI IsGlobal (HQ.HashQualified Name) to NamesI IsGlobal [(RawQuery, ErrorMessageOrName)] where the following type aliases are defined:

type ErrorMessageOrValue a = Either (P.Pretty P.ColorText) a
type ErrorMessageOrName = ErrorMessageOrValue (HQ.HashQualified Name)
type RawQuery = String

This change is required so that a batch of names can be handled with the original raw query string available to be printed as headings in the final output and so error messages in parsing a raw query/arg can be printed if necessary for each query.

Updates Unison.CommandLine.InputPatterns to take multiple names and produce the modified NamesI.

Updates Unison.Codebase.Editor.HandleInput to handle the modified NamesI by mapping over each name/hash query in the batch.

Adds BatchedOutput and IndentedOutput constructors to Unison.Codebase.Editor.Output to help format the output of names such that each raw name query becomes a header for each listing indented below it.

Interesting/controversial decisions

Include anything that you thought twice about, debated, chose arbitrarily, etc.
What could have been done differently, but wasn't? And why?

1

I considered adding a BatchedInput constructor to Unison.Codebase.Editor.Input (Input), but it proved difficult to reach into Cli () monad produced by code that handles input to properly format a sequence of Inputs. The benefit of this approach may have been a more reusable/externalized approach to adding multiple argument handling to other input handlers.

2

For simplicity and consistency, even when a single name is queried, the output of names now includes a heading for the raw name query:

scratch/main> names max

  max:
    Term
    Hash:   ##Float.max
    Names:  builtin.Float.max

Test coverage

Existing transcript tests cover names and debug.names.global queries for single argument cases. Cases for handling multiple arguments were added to these tests. Existing transcript tests also cover the help output when using e.g. ? names.

Loose ends

I want to cover adding multiple args support to list/ls in a new issue (to be created).

[aryairani]

Hi @xmbhasin this is great!

Also, the thorough PR description is very helpful. I have some requests/suggestions:

Instead of visually grouping the results with headings, which raises interesting/controversial decision 2, we can just put them back to back, i.e.:

scratch/main> names max +

  Term
  Hash:   ##Float.max
  Names:  builtin.Float.max


  Terms
  Hash:   ##Float.+
  Names:  builtin.Float.+

  Hash:   ##Int.+
  Names:  builtin.Int.+

  Hash:   ##Nat.+
  Names:  builtin.Nat.+

or even without the extra blank line when grouping, I'm unsure one way or the other whether it would be helpful or unhelpful. (cc @hojberg ?).

That way we can also avoid introducing the new Output constructors BatchedOutput and IndentedOutput. They do make sense in this case, but I'm not convinced yet that they pay their way in terms of the codebase overall.

I believe the input constructor can then also go back to NamesI IsGlobal [HQ.HashQualified Name] in that case. But even if we kept the headings, I would still suggest something like ([P.Pretty P.ColorText], [HQ.HashQualified Name]) instead of the three new type aliases.

Also, once we merge this, I think we can go ahead and close #3557, since that's just about names; but we can open additional tickets for each additional command we want to add multi-argument support to.

Let me know if I misunderstood something; I'm also happy to chat design here or on Discord whenever's good.

[xmbhasin]

@aryairani Thanks for taking a look.

I agree with making a new ticket for ls after.

The related change for list/ls to take multiple arguments should also be considered here as it should follow a similar design/pattern.

I've got some draft changes that works as following for ls:

scratch/main> ls builtin.Either invalid builtin.Handle
	builtin.Either:
		1. Left  (a -> Either a b)
		2. Right (b -> Either a b)

	invalid:
		nothing to show

	builtin.Handle:
		3. toText (Handle -> Text)

Without the headings this might look like this:

scratch/main> ls builtin.Either invalid  builtin.Handle

	1. Left  (a -> Either a b)
	2. Right (b -> Either a b)

	nothing to show

	3. toText (Handle -> Text)

I don't think the output with headings when just one arg is passed to names or ls looks bad and it keeps it consistent if we do decide to keep headings. I do think the output looks a bit muddy without headings when there are 3 or more arguments and some of them cannot be parsed/handled correctly.

Let me know if this helps convince.

[xmbhasin]

If we group successful and unsuccessful queries, we could print a header before each group instead, which should be relatively easy to do without introducing new Output constructors.

Something like this:

scratch/main> names max /invalid1 /invalid2 +

I successfully parsed and searched for the following name queries: max +

Term
Hash:   ##Float.max
Names:  builtin.Float.max


Terms
Hash:   ##Float.+
Names:  builtin.Float.+

Hash:   ##Int.+
Names:  builtin.Int.+

Hash:   ##Nat.+
Names:  builtin.Nat.+


I failed to search for the following name queries: /invalid1 /invalid2

 /invalid1 is not a well-formed name, hash, or hash-qualified name. I expected something like `foo`,
`#abc123`, or `foo#abc123`.


 /invalid2 is not a well-formed name, hash, or hash-qualified name. I expected something like `foo`,
`#abc123`, or `foo#abc123`.

With a single name query, we would have:

scratch/main> names max

I successfully parsed and searched for the following name queries: max

Term
Hash:   ##Float.max
Names:  builtin.Float.max

If there is only one query, it would be easy to omit the header with this approach.

The type of the NamesI constructor would still need to be NamesI IsGlobal [(String, Either (P.Pretty P.ColorText), (HQ.HashQualified Name)) to print these headers.

If we opt to have no headers at all then I think the regrouping/reordering would be a bit confusing.