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 /invalid1 /invalid2 + Boolean foo baz

  'max':
  Hash          Kind   Names
  ##Float.max   Term   lib.builtins.Float.max

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

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

  '+':
  Hash        Kind   Names
  ##Float.+   Term   lib.builtins.Float.+
  ##Int.+     Term   lib.builtins.Int.+
  ##Nat.+     Term   lib.builtins.Nat.+

  'Boolean':
  Hash            Kind   Names
  #idl63c82kf#0   Term   a.baz.Boolean
  #56fi1cmq3u     Term   aa.baz,
                         another.Boolean,
                         bb.baz,
                         cc.baz,
                         dd.baz
  ##Boolean       Type   lib.builtins.Boolean
  #cmihlkoddu#0   Term   z.baz.Boolean

  'foo':
  😶
  I couldn't find anything by that name.

  'baz':
  Hash          Kind   Names
  #idl63c82kf   Type   a.baz
  #u1qsl3nk5t   Term   a.baz, b.baz, c.baz, d.baz
  #56fi1cmq3u   Term   aa.baz,
                       another.Boolean,
                       bb.baz,
                       cc.baz,
                       dd.baz
  #00kr10tpqr   Term   xyz.baz
  #cmihlkoddu   Type   z.baz

Note that due to partial failure (some name queries are invalid or unknown/empty), the entire names command is considered a failure.

The output is coloured to show success/failure of each name query at a glance

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.

Updates Output type in Unison.Codebase.Editor.Output so that the ListNames and GlobalListnames constructors take an additional String value that is the name query for printing headers.

Updates function listOfNames in Unison.CommandLine.OutputMessage to print a compact table of hashes, hash kinds, and names. Sorts the names column alphabetically and orders rows by the names column.

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

When a single name is queried, the output of names still includes a heading for the raw name query:

scratch/main> names max

  '+':
  Hash              Kind        Names
  builtin.Float.+   ##Float.+   Term
  builtin.Int.+     ##Int.+     Term
  builtin.Nat.+     ##Nat.+     Term

2

The Kind column shows whether the hash refers to a term or a type. Kind may be an overloaded word here, but I wasn't aware of a better word.

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. Cases exercising the sorting of the tabular output were also added. 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).

During design discussion we wanted to put the Names column first, but because of an outstanding pretty printing bug we decided to defer this. See #5550.

Evolution

Originally, the output of names remained similar to the original format but with headers for the name queries. This was changed to provide a more compact table-oriented format after design discussion.

Additionally, new Output constructors were initially added (BatchedOutput and IndentedOutput) to enable adding such headers without touching the original Output constructor signatures. This was changed to reduce the complexity overhead from the new constructors.

[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.

[aryairani]

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.

Hi @xmbhasin, sorry for the wait.

This looks good!

Let's do this (grouping successful and unsuccessful queries), and omit the header if there is only one query.

---

As an optional bonus, or a later effort, we were thinking something like this:

"max":                      hash
builtin.Float.max           ##Float.max

"+":                        hash
builtin.Float.+             ##Float.+
builtin.Int.+               ##Int.+
builtin.Nat.+, another.+    ##Nat.+

with the header line in grey. You can (hopefully) use the column2Header helper function for this (though I wouldn't be surprised if it had some issue with long or split lines on the left...)

Indented is also okay:

"max":                      hash
  builtin.Float.max           ##Float.max

"+":                        hash
  builtin.Float.+             ##Float.+
  builtin.Int.+               ##Int.+
  builtin.Nat.+, another.+    ##Nat.+

---

How does this sound?

[xmbhasin]

As discussed, I'll try to achieve the following example format:

scratch/main> names max /invalid1 /invalid2 + Boolean foo baz

  'max':
  Hash          Kind   Names
  ##Float.max   Term   lib.builtins.Float.max

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

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

  '+':
  Hash        Kind   Names
  ##Float.+   Term   lib.builtins.Float.+
  ##Int.+     Term   lib.builtins.Int.+
  ##Nat.+     Term   lib.builtins.Nat.+

  'Boolean':
  Hash            Kind   Names
  #idl63c82kf#0   Term   a.baz.Boolean
  #56fi1cmq3u     Term   aa.baz,
                         another.Boolean,
                         bb.baz,
                         cc.baz,
                         dd.baz
  ##Boolean       Type   lib.builtins.Boolean
  #cmihlkoddu#0   Term   z.baz.Boolean

  'foo':
  😶
  I couldn't find anything by that name.

  'baz':
  Hash          Kind   Names
  #idl63c82kf   Type   a.baz
  #u1qsl3nk5t   Term   a.baz, b.baz, c.baz, d.baz
  #56fi1cmq3u   Term   aa.baz,
                       another.Boolean,
                       bb.baz,
                       cc.baz,
                       dd.baz
  #00kr10tpqr   Term   xyz.baz
  #cmihlkoddu   Type   z.baz