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

unison transcript unison-src/transcripts-round-trip/main.md is failing on Windows #2940

Closed
Tracked by #598
aryairani opened this issue Feb 19, 2022 · 3 comments · Fixed by #2943
Closed
Tracked by #598

unison transcript unison-src/transcripts-round-trip/main.md is failing on Windows #2940

aryairani opened this issue Feb 19, 2022 · 3 comments · Fixed by #2943
Assignees
Labels

Comments

@aryairani
Copy link
Contributor

aryairani commented Feb 19, 2022

```unison
    docTest2 : Doc2
    docTest2 =
      {{ # Full doc body indented

        ``` raw
        myVal1 = 42
        myVal2 = 43
        myVal4 = 44
        ```

        ``` raw
        indented1= "hi"
        indented2="this is two indents"
        ```

        I am two spaces over }}

  You can edit them there, then do `update` to replace the
  definitions currently in this namespace.

```
```ucm
.> load scratch.u

  I found and typechecked these definitions in scratch.u. If you
  do an `add` or `update`, here's how your codebase would
  change:

    ⍟ These names already exist. You can `update` them to your
      new definition:

      docTest2 : Doc2

```
@ChrisPenner
Copy link
Contributor

ChrisPenner commented Feb 22, 2022

Investigated further, this is caused by carriage returns being inserted at the end of lines within codeblocks when pretty-printing on windows.

We'll probably want to either strip those out when parsing, or avoid printing them in the first place (but that might wreak havoc for some windows text editors).
My thought would be to strip them out when parsing and add them back in when printing.

Alternatively we can represent the lines of code-blocks semantically in the Doc AST and avoid the problem altogether, but that's likely more work than its worth. 🤷🏼‍♂️

I'll dig into the parsing and printing code and see what's easiest.

@aryairani
Copy link
Contributor Author

aryairani commented Feb 22, 2022

There should be a text mode for the file handles that does the right thing automatically (i.e. on windows, turns CRLF into LF on read, and turns LF into CRLF on write; and does no translation on unix?). We don't want to bake the solution into our parsing rules.

@ChrisPenner
Copy link
Contributor

Ah, yeah that's a much better solution. I'll adjust the file Handle newline mode in the parser.

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

Successfully merging a pull request may close this issue.

2 participants