unisonweb / unison

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

Doc bugs mega issue #4610

Closed SystemFw closed 10 months ago

SystemFw commented 10 months ago

I found a lot of bugs with Doc handling when doing writeups for AoC. They all appeared on trunk, as of December 20th or smth (haven't had the time to test again).

They aren't super easy to reproduce, most of them appear when editing docs, I've tried to link to example docs that should display the bugs when edited, but they are nondeterministic, or at least I couldn't find a clear pattern.

  1. [x] 1. ~A Doc in the scratch file keeps being marked for update even though it hasn’t changed. Most of the following bugs happen in conjunction with this one: day 10~
  2. [ ] 2. @source{foo} gets transformed into {{ docSource [docSourceElement (docEmbedTermLink 'foo) []] }}: day 10
  3. [x] 3. Text inside a '''text''' block gets shifted like 40 chars to the right. This keeps happening and messing up my ascii art, and I have to keep going back to fix it: day 10
  4. [x] 4. a `thing` text will sometimes get transformed into a ''text'', which messes things up if the text has a single tick already, eg. `'{IO} a'`, becomes '''{IO} a'' and everything gets messed up. This happened with the same doc, where calling update twice fixed it: day 14
  5. [x] 5. ~A term only has a hash on share, but it's displayed with a name locally in ucm (the term is List.foldBalanced): day 5~ (Chris) Now hashes will show in both if the names are missing.
ChrisPenner commented 10 months ago

A term only has a hash on share, but it's displayed with a name locally in ucm (the term is List.foldBalanced)

I suspect this will be fixed in #4579

ChrisPenner commented 10 months ago

Starting to look into these, I think 1. is likely caused by:

  1. Add a doc to the codebase, it'll resolve {{ }} into the hashes for docUntitledSection from your current base version.
  2. Upgrade Base
  3. edit the doc, if you're lucky it'll find names for the old docUntitledSection from somewhere (previously: the global PPE fallback), then will print with {{ }} again. But now, these will desugar to the NEW docUntitledSection from the new base, thus signalling an update.

As for what to do about that, I'm not 100% sure, but I think the "hot-swappable doc combinator syntax" idea was probably a mistake. You shouldn't need a copy of base to write docs, but even if you do, upgrading base shouldn't break your doc rendering 🤔

ChrisPenner commented 10 months ago

Number 2 seems to have the docSource wrapped in a transclusion node that just writing @{foo} doesn't generate. I can't reproduce it yet, I'll dig through the parser and see if I can find anywhere that'd introduce one.

Edit: couldn't find any; so I'll leave that one until someone is able to find a reproduction 🤔

ChrisPenner commented 10 months ago

Chatted with the team about Number 1, update propagation should be preventing that in most cases, in the worst case there's a couple extra updates and that's probably fine. We can revisit if it comes up often.

I split off this issue to track: https://github.com/unisonweb/unison/issues/4652

ChrisPenner commented 10 months ago

3 is easily reproducible, it's caused by the pretty-printer "nicely" indenting all aspects of a doc for readability, including text literals, but then this adds a bunch of spaces.

I think we'll just have to have raw text-literals be fully de-dented, but this doesn't seem possible in the current iteration of the pretty-printer so I'm checking with @runarorama about it.

Another option would be to detect and strip leading whitespace from all lines of a text literal, but I don't like that because it makes some otherwise valid text-literals unrepresentable.

ChrisPenner commented 10 months ago

4 seems to happen for me every time; It's strange to me that we support both syntaxes, maybe we should JUST support the backtick syntax. It's more compliant to markdown anyways.

ChrisPenner commented 10 months ago

All except for 2 have resolutions (or at least are well-understood), so I pulled out 2 here: https://github.com/unisonweb/unison/issues/4660

And will close this one :)