search

unisonweb / website

Main Unison website
MIT License
8 stars 5 forks source link

Docs typo - tried to fix and failed #28

Open drshade opened 2 years ago

drshade commented 2 years ago

Hello Unison Team!

I was trying to fix a typo I found while going through the "learn/fundamentals/control-flow/pattern-matching2" examples - specifically the slayHydra functions under the as-patterns section. Opening the website above you can see the docs are not correct, and the second slayHydra does not use the "@" symbol. So I thought I would try to fix it myself - and managed to pull and fork the docs:

$ ucm --codebase-create codebase
[Snip]
.> pull https://github.com/unisonweb/website:.learn ._learn
[Snip]
.> cd _learn.fundamentals.controlFlow
._learn.fundamentals.controlFlow> display _asPatterns
[Shows the bug! slayHydra is incorrect in the second form]
._learn.fundamentals.controlFlow> edit _asPatterns
[Shows slayHydra is correct in the second form...]

So that seems odd to me - I thought maybe initially it as Unison being clever and maybe both functions are identical so they effectively hash to the same content, so I tried to modify the second example more invasively by adding an extra function parameter to slayHydra - but to no avail. It does not seem to update (e.g. view _asPatterns vs. display _asPatterns).

Even more weird is when I go display patternMatching2 the code examples under the "As-patterns" actually show the placeSetting functions instead of the slayHydra functions!! So now I'm convinced there is a bug somewhere - but will need some guidance around this as it seems quite core and probably out of my weight class for now!

All the best, Tom

rlmark commented 2 years ago

Hi @drshade! Thank you so much for pointing this out! This is an artifact of Unison terms and functions being rendered in a more opinionated way than what might initially be expressed. The pretty printer will sometimes normalize a function in a way that is different from what we initially write. One thing I like to try to address this is to include the problematic function with the @source{myFunction} docs element instead of via the fenced codeblocks. The rendering for that is typically a little more faithful. It looks like this might be a case where that would be a viable fix. If you'd like, I can make the fix, and if you send over your authorship / license info I can link your info to the function!

drshade commented 2 years ago

Thanks @rlmark ! I would love to take a crack at fixing it myself and will update here with findings.

rlmark commented 2 years ago

Awesome! Let me know if you have any questions!

drshade commented 2 years ago

Hi @rlmark !

I tried to follow through the pull-request mechanism described, including some of the author / license requirements too and this is what I have come up with:

.> pull-request.create https://github.com/unisonweb/website:.learn git@github.com:drshade/unison-codebase.git:.prs._learn

  The changes summarized below are available for you to review, using the following command:

    pull-request.load https://github.com/unisonweb/website:.learn git@github.com:drshade/unison-codebase.git:.prs._learn

  Updates:

     fundamentals.controlFlow._asPatterns : Doc
     ↓
     fundamentals.controlFlow._asPatterns : Doc
     +  rlmark              : base.Author
     +  authors.drshade     : base.Author
     +  unisoncomputing2021 : License

    There were 7 auto-propagated updates.

  Added definitions:

     fundamentals.controlFlow.metadata.authors.drshade          : base.Author
     fundamentals.controlFlow.metadata.copyrightHolders.drshade : CopyrightHolder
     fundamentals.controlFlow.metadata.licenses.drshade2022     : License
     fundamentals.controlFlow.metadata.authors.drshade.guid     : GUID
     fundamentals.controlFlow._asPatterns.slayHydra1            : Nat
                                                                -> Hydra
                                                                -> Optional Hydra (+2 metadata)
     fundamentals.controlFlow._asPatterns.slayHydra2            : Nat
                                                                -> Hydra
                                                                -> Optional Hydra (+2 metadata)

     patch fundamentals.controlFlow.patch (added 1 updates)

From a _asPatterns perspective I wasn't sure how to keep the function name the same for both snippets of slayHydra, so I eventually just decided to name them slayHydra1 and slayHydra2 - this is probably wrong and i'm sure you could guide me to a better way to keep it consistent for the reader. I initially tried all sorts of combinations of @source, including wrapping in triple-ticks, double-ticks, single quote, etc to try and embed each function seperately - but all of those gave me problems, so eventually just ended up lifting the two functions to the namespace level.

(On the whole however I'm enjoying learning Unison and the process so I hope you can bear with me!)

All the best, Tom