unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.78k stars 270 forks source link

diff.namespace produces malformed numbered args and `view` doesn't work for full hashes #4575

Open pchiusano opened 10 months ago

pchiusano commented 10 months ago

The history browsing experience has never been great, but it could be improved to "functional" with these fixes -

@unison/json/main> history

  Note: The most recent namespace hash is immediately below this message.

  ⊙ 1. #thej7504d0

    + Adds / updates:

      ReleaseNotes

  ⊙ 2. #j8os53s31q

    + Adds / updates:

      Decoder.object.optionalAt.doc Decoder.object.optionalAt!.doc Decoder.optional.doc
      ReleaseNotes

  ⊙ 3. #lto8kqepal

    + Adds / updates:

      Decoder.array Decoder.array.at!.doc Decoder.array.doc Decoder.array! Decoder.array!.doc
      Decoder.DecodingFailure.doc Decoder.DecodingFailure.raiseFailure
      Decoder.DecodingFailure.toBaseFailure Decoder.DecodingFailure.toBaseFailure.doc
      Decoder.DecodingFailure.toText Decoder.DecodingFailure.toText.doc Decoder.doc Decoder.object
      Decoder.object.doc Decoder.object.optionalAt.tests.absent
      Decoder.object.optionalAt.tests.present Decoder.object.sum.doc Decoder.object!
      Decoder.object!.doc Decoder.optional.doc Decoder.optional!.doc Decoder.reraise Decoder.run
      Decoder.run.doc Decoder.tests.ex2 Decoder.tests.ex3 Decoder.tryRun.doc
      Decoder.tryRun.noTrailing.doc Readme

  ⊙ 4. #vl1f4q0rm2
@unison/json/main> diff.namespace 3 1

  Updates:

    1. Decoder.object.optionalAt.doc : Doc
       ↓
    2. Decoder.object.optionalAt.doc : Doc

    3. Decoder.optional.doc : Doc
       ↓
    4. Decoder.optional.doc : Doc

    5. ReleaseNotes : Doc
       ↓
    6. ReleaseNotes : Doc

  Added definitions:

    7. Decoder.object.optionalAt!.doc : Doc

@unison/json/main> view 1 2 7

  ⚠️

  #lto8kqepal0uuqolsag0lcpetke61ff8lqoeoenl4n0h95gf941g7618jhjscg63n5c7ur97k13kauh8e7gcq5moh8nu5s6fk03s8t8:.Decoder.object.optionalAt.doc#t90rcor444tllvpdhh05ehm4p5mnnhfa5tj78s0ath04svidp5lvvqapsr07ujtrnhidcscpkbtktj9vk5kgeebuq076d2eeb00isvo
  is not a well-formed name, hash, or hash-qualified name. I expected something like `foo`,
  `#abc123`, or `foo#abc123`.

@unison/json/main> view 1

  ⚠️

  #lto8kqepal0uuqolsag0lcpetke61ff8lqoeoenl4n0h95gf941g7618jhjscg63n5c7ur97k13kauh8e7gcq5moh8nu5s6fk03s8t8:.Decoder.object.optionalAt.doc#t90rcor444tllvpdhh05ehm4p5mnnhfa5tj78s0ath04svidp5lvvqapsr07ujtrnhidcscpkbtktj9vk5kgeebuq076d2eeb00isvo
  is not a well-formed name, hash, or hash-qualified name. I expected something like `foo`,
  `#abc123`, or `foo#abc123`.

uh8e7gcq5moh8nu5s6fk03s8t8                                                                                                       

  ⚠️

  The following names were not found in the codebase. Check your spelling.
    #lto8kqepal0uuqolsag0lcpetke61ff8lqoeoenl4n0h95gf941g7618jhjscg63n5c7ur97k13kauh8e7gcq5moh8nu5s6fk03s8t8

@unison/json/main> view 2

  ⚠️

  #thej7504d0msvq3vpea10ssnuhmfumjje7j2co4gkr4cob5n7vrufo88tqe4dubopme25ufnop0jkgrab24oo595gerajoqr5aur8r8:.Decoder.object.optionalAt.doc#r9n17sa7l9jp1j54btplg677dqn1d8uvv7pcgnl5thbs2j6lgopog5tbkdkq6q9kf9jdqgm5bdpoa2ujk0cfhiurlq703b8vo7umue8
  is not a well-formed name, hash, or hash-qualified name. I expected something like `foo`,
  `#abc123`, or `foo#abc123`.

@unison/json/main> debug.numberedArgs

  1. #lto8kqepal0uuqolsag0lcpetke61ff8lqoeoenl4n0h95gf941g7618jhjscg63n5c7ur97k13kauh8e7gcq5moh8nu5s6fk03s8t8:.Decoder.object.optionalAt.doc#t90rcor444tllvpdhh05ehm4p5mnnhfa5tj78s0ath04svidp5lvvqapsr07ujtrnhidcscpkbtktj9vk5kgeebuq076d2eeb00isvo
  2. #thej7504d0msvq3vpea10ssnuhmfumjje7j2co4gkr4cob5n7vrufo88tqe4dubopme25ufnop0jkgrab24oo595gerajoqr5aur8r8:.Decoder.object.optionalAt.doc#r9n17sa7l9jp1j54btplg677dqn1d8uvv7pcgnl5thbs2j6lgopog5tbkdkq6q9kf9jdqgm5bdpoa2ujk0cfhiurlq703b8vo7umue8
  3. #lto8kqepal0uuqolsag0lcpetke61ff8lqoeoenl4n0h95gf941g7618jhjscg63n5c7ur97k13kauh8e7gcq5moh8nu5s6fk03s8t8:.Decoder.optional.doc#n7fu34sa73c347ifde10aa1u2sog86kd7u52u313bsau50h79ijv0l9lsgn76442mhj5eogj9u6f40u7vqm2q0arqsgb28jhsq7mhto
  4. #thej7504d0msvq3vpea10ssnuhmfumjje7j2co4gkr4cob5n7vrufo88tqe4dubopme25ufnop0jkgrab24oo595gerajoqr5aur8r8:.Decoder.optional.doc#hfj51u24tgapuja8vthtrimb8ti9427uo0bmunajo7opljutodqh5ls3luqrgo1dhb27tq7qtomg1392d8b6mmbnfs1l1789crq4m30
  5. #lto8kqepal0uuqolsag0lcpetke61ff8lqoeoenl4n0h95gf941g7618jhjscg63n5c7ur97k13kauh8e7gcq5moh8nu5s6fk03s8t8:.ReleaseNotes#mag2oqqbot96a0qqnkoc15nl6bofro9gkv1n361sap4sgrngba3u1vfq08q1bufcr6cs4vvgk1ddvs0fonmli1stqotidesm17njvog
  6. #thej7504d0msvq3vpea10ssnuhmfumjje7j2co4gkr4cob5n7vrufo88tqe4dubopme25ufnop0jkgrab24oo595gerajoqr5aur8r8:.ReleaseNotes#bjo2p8gfq504d3j8pbpcnmrnh7nqu6ac6bneo7mq41u58sdl5s7m6bc511a8n3m06ree1tg4a2im8baugacca0h4ug7q4ms62cibke0
  7. #thej7504d0msvq3vpea10ssnuhmfumjje7j2co4gkr4cob5n7vrufo88tqe4dubopme25ufnop0jkgrab24oo595gerajoqr5aur8r8:.Decoder.object.optionalAt!.doc#rp4g1vjmi16e37ipem1ang2k16t8lgj21h3a6e3jpv2dfamj16mkftae0qfoot7d94jhj2uergo9fm891nummgetar6jriq8c1ktono

This is not great since you typically want to be able to view the definitions in the diff.

Sadly view does not seem to work for full hashes. This used to work, I'm curious what happened.

mitchellwrosen commented 10 months ago

@pchiusano I think view does work with full term/decl hashes, but not namespace hashes. Is that what used to work?

aryairani commented 10 months ago

@mitchellwrosen Github is hiding the lower part of the log but you can scroll down, it looks like diff.namespace is constructing the numbered args as hash-only instead of hash-qualified names. (Edit: this is expected)

aryairani commented 10 months ago

@pchiusano view should work with full hashes but only if the hash is in your current namespace; I'm guessing they're not, they're just in the older namespaces?

aryairani commented 10 months ago

It's tricky because we could change view to be able to display hashes that aren't in your namespace, but where do we get the names for the dependencies? view only really makes sense in the context of a specific namespace. We could try it in the current namespace, but it might just be hashes. Or we could have a separate command for it view.in #nshash #defnhash :shrug:

pchiusano commented 10 months ago

For a start, I'd just have view work for hashes not in your namespace, and just use the current namespace to provide names. In the worst case scenario you see some hashes, but it's still handy - you shouldn't have to switch namespaces just to view a hash.

Even better would be if diff.namespace suggested diff.term or diff.type, which would Just Work when passed any two numbered arguments presented in the diff.namespace output. The numbered args for diff.namespace could be something with a bit more information, which diff.term could use to find names.

mitchellwrosen commented 10 months ago

Oh! I see: diff.namespace is outputting numbered args like this:

and view doesn't like those.

aryairani commented 10 months ago

@mitchellwrosen No, I think it's outputting them like <term-hash>, but view likes those, but only if <term-hash> appears in the current namespace.

mitchellwrosen commented 10 months ago

Oh? Am I looking at the wrong output above? Where's the <term-hash>?

Also, I can't reproduce the behavior of view requiring the hash you give it to exist in the current namespace. Do you have a transcript for that?

aryairani commented 10 months ago

@mitchellwrosen Sorry, I misread the output above, you're right.

aryairani commented 10 months ago

So actually it's great that it tells us what namespace the term existed in, because then we can make our PPE. But the whole PPE situation (this issue, ongoing work, etc.) feels overly complicated. I wonder if we can come up with a theory that's simple and easy to implement. Something that includes scratch files, namespaces, paths, hashes, lol.

For the short term, maybe we could make view be able to parse that kind of argument? [[<nshash/causalhash>:][.]<path>][#term-hash] and then ignore parts of it?