Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Tracked by #4899
pchiusano opened this issue Jan 4, 2024 · 10 comments

Comments

@pchiusano
Copy link
Member

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
Copy link
Member

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

@aryairani
Copy link
Contributor

aryairani commented Jan 4, 2024

@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
Copy link
Contributor

@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
Copy link
Contributor

aryairani commented Jan 4, 2024

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 🤷

@pchiusano
Copy link
Member Author

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
Copy link
Member

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

  • <causal-hash>:<name><term-hash>

and view doesn't like those.

@aryairani
Copy link
Contributor

aryairani commented Jan 10, 2024

@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
Copy link
Member

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
Copy link
Contributor

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

@aryairani
Copy link
Contributor

aryairani commented Jan 10, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants