unisonweb / unison

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

Fix roundtrip bug in Bytes.splitAt.doc #5121

Closed neduard closed 5 days ago

neduard commented 6 days ago

Closes https://github.com/unisonweb/unison/issues/4727

Overview

Wraps the printing of BytesLiteral in a F.Group this removes the space between elements.

NOTE I am not 100% sure this is the right fix. I haven't traced exactly how the Pretty datatype gets converted to a String. I just noticed we use group in other cases where we don't want separation eg:

prettyRemoteBranchInfo :: (URI, ProjectName, ProjectBranchName) -> Pretty
prettyRemoteBranchInfo (host, remoteProject, remoteBranch) =
  -- Special-case Unison Share since we know its project branch URLs
  if URI.uriToString id host "" == "https://api.unison-lang.org"
    then
      P.group $
        "https://share.unison-lang.org/"
          <> prettyProjectName remoteProject
          <> "/code/"
          <> prettyProjectBranchName remoteBranch
  ...

Without the fix we would get this failure:

    The transcript failed due to an error in the stanza above. The error is:

      This looks like a function call, but with a Bytes where the function should be.  Are you missing an operator?

        302 | fix_4727 = {{ `` 0xs 900dc0ffee `` }}

Loose ends

I added the test in reparses-with-same-hash.u mainly because there isn't a dedicated test for TermPrinter.hs (there is only one for TypePrinter.hs) I'm wondering if it would be better add it though?

aryairani commented 5 days ago

I think this is probably the right fix. Just need to merge in trunk because there was a conflict with the other PR. (I'm trying as well, but don't mind if you beat me to it.)