unisonweb/unison

How does the pretty printer decide to qualify names?

Opened this issue · 2 comments

How do we explain the behaviour of the pretty printer in this doc?
https://share.unison-lang.org/@systemfw/aoc-2024/code/main/latest/terms/day08/doc

solve : Boolean -> Text -> Nat
solve extend input =
  use Boolean not
  use List ++ +:
  use Universal neq
  use lib.base.data.List filter flatMap
  points = Point.parse input
  inLine =
    points
      |> filter (at2 >> neq ?.)
      |> lib.base.data.List.groupMapReduce at2 (p -> [at1 p]) (++)
      |> Map.values
      |> flatMap
           (antennas ->
             antennas
               |> flatMap
                    (a ->
                      lib.base.data.List.map (Tuple.pair a) antennas)
               |> filter (as -> neq (at1 as) (at2 as))
               |> flatMap (as -> [as, Tuple.swap as]))
  space = lib.base.data.List.toMap points
  getAntinodes = cases
    (p, p') ->
      _ = "diff = p' - p"
      diff = p' |> move (Point.scale -1 p)
      go antinodes p =
        use List :+
        antinode = move p diff
        if not (Map.contains antinode space) then antinodes
        else go (antinodes :+ antinode) antinode
      if not extend then go [] p' |> List.take 1 else p' +: go [] p'
  inLine
    |> flatMap getAntinodes
    |> lib.base.data.List.distinct
    |> List.size

As you can see, some List functions appear unqualified, like filter. Others are only qualified by type, like List.size. Others still are ully qualified, like lib.base.List.map . names of the fully qualified names reports a single hash.

Local display does a bit better, the qualified names aren't fully qualified, although It's still unclear to me how it decides when to qualify in the first place.

solve : Boolean -> Text -> Nat
solve extend input =
  use Boolean not
  use List ++ +: filter flatMap
  points = Point.parse input
  inLine =
    points
      |> filter (at2 >> (!==) ?.)
      |> groupMapReduce at2 (p -> [at1 p]) (++)
      |> Map.values
      |> flatMap
           (antennas ->
             antennas
               |> flatMap (a -> List.map (Tuple.pair a) antennas)
               |> filter (as -> at1 as !== at2 as)
               |> flatMap (as -> [as, Tuple.swap as]))
  space = Map.fromList points
  getAntinodes = cases
    (p, p') ->
      _ = "diff = p' - p"
      diff = p' |> move (Point.scale -1 p)
      go antinodes p =
        use List :+
        antinode = move p diff
        if not (Map.contains antinode space) then antinodes
        else go (antinodes :+ antinode) antinode
      if not extend then go [] p' |> List.take 1 else p' +: go [] p'
  inLine |> flatMap getAntinodes |> distinct |> List.size

We use a heuristic, which iirc, for the local version, is that it qualifies the minimum number of segments necessary to be uniquely determine the reference.

So:

x.a = "hello"
y.a = "hello"
p.b = "goodbye"
q.b = "goodbye2"
f z = z
prog = x.a ++ q.b
prog2 g = f (x.a ++ q.b ++ g)
issue5490/main> add
issue5490/main> move.term f g
issue5490/main> view prog prog2

is expected to produce:

prog = a ++ q.b -- a is not ambiguous, because x.a and y.a are the same
prog2 g = .g (a ++ q.b ++ g)

although currently prog2 prints incorrectly, with a fix in progress.

I agree the heuristic isn't great, but we haven't come up with something better.

This seems like a bug with the Share rendering, it's including more segments than it should. The local and Share rendering should generally be the same.

I think I've said this before, but I'd also be in favor of just never showing more than two segments on Share. You can always hover or click through. Once you have blah.foo.bar.quaffle.List.map, your eyes glaze over anyway and it's hard to read.