[#164] Improve Windows support

[Sorokin-Anton]

Description

When I tried to test xrefcheck on windows locally, I've found following problems:

Problem: it requires some dynamic libraries to run, and stack install place them in directory that is not on PATH by default
Solution: add this information and possible workarounds to README.md, create issue is stack

---

Problem: most Windows shells are not supporting utf8 by default. Sometimes printing of reports raise an error. hPutChar: invalid argument (Invalid argument)

Solution: use function withCP65001 from code-page package which
sets correct codepage on Windows and do nothing on other OSs

---

Problem: in markdown links, '/' should always be used as path separator ,
e.g. GitHub renderer is not threating link [x](a\b.md) as a link
to file in dir a
We use OS-dependent file separators everywhere, so
e.g. canonizeLocalRef "./a.md"="./a.md"` on Windows

Solution: replace '' to '/' while constructing repo tree if needed
and then use only '/', preferring functions from System.FilePath.Posix.
We can do this, since 'a/b' and a\b are equivalent paths on Windows

---

Also I've added github actions workflow that runs stack test xrefcheck:xrefcheck-tests and golden tests (this required modification of some tests)

Related issue(s)

Fixes #164

✅ Checklist for your Pull Request

Ideally a PR has all of the checkmarks set.

If something in this list is irrelevant to your PR, you should still set this
checkmark indicating that you are sure it is dealt with (be that by irrelevance).

Related changes (conditional)

• Tests

  • If I added new functionality, I added tests covering it.
  • If I fixed a bug, I added a regression test to prevent the bug from
    silently reappearing again.

• Documentation

  • I checked whether I should update the docs and did so if necessary:
    • README
    • Haddock

• Public contracts

  • Any modifications of public contracts comply with the Evolution
    of Public Contracts policy.
  • I added an entry to the changelog if my changes are visible to the users
    and
  • provided a migration guide for breaking changes if possible

Stylistic guide (mandatory)

• My commits comply with the policy used in Serokell.
• My code complies with the style guide.

[Sorokin-Anton]

Ok, now I need to think how to add case-sensetivity on Windows

UPD: after rebase on #174, case sensetivity works correctly

[Martoon-00]

Looks good to me, leaving just a few minor comments.

I'll try to wrap my head around possible pitfalls for more and maybe will give another review later.

[Martoon-00]

In Main.hs module we use withUtf8, which sort of tries to switch everything to UTF-8 so that the developer does not need to worry about all the changes that must be made.

How do you think, would it be justified to create an issue in their GitHub? (Actually, this is a Serokell's package, so it could be just an improvement suggestion)

[Sorokin-Anton]

Good idea. Created issue in that repo, also moved withCP65001 to Main to keep such things in one place

[Martoon-00]

Awesome 👍

[Martoon-00]

We use defConfigText in xrefcheck dump-config, will it be fine if we eventually print that text without \rs?

[Sorokin-Anton]

Checked, this is not a problem for yaml parser

[Martoon-00]

How about the user that will try to open and edit that file? Won't they see everything displayed in one line (at least in some text editors)?

UPD: or maybe not in one line, rather without caret returns.

[Martoon-00]

Pacman does not feel too well? 🧌

[Sorokin-Anton]

I just recorded my shell history, and then realized it's the way

[Martoon-00]

It's just a bit of torture to see a mention of some magic without an explanation of why it is needed / why it works 😅

On the other hand, if all the possible explanation is "doing it once fails with X error, but somehow repeat helps both locally and in CI, no one knows why", then maybe it's really not necessary to provide.

[Sorokin-Anton]

Oh, looks like ci action fails because diff is now space-sensetive, so does not like cr-s. Locally on Windows everything passes (no cr-s) , for me it's still a question where and why cr-s appear. Luckily there is diff option specially for that

[Martoon-00]

Oh, looks like ci action fails because diff is now space-sensetive

I would take a more thorough look at this, as it may point to a real UX issue of xrefcheck that will rise for some users.

I have two thoughts:

1. AFAIR when I installed git in Windows, it asked me which line ending I want to use and whether do I want to replace non-matching line endings automatically.

   And looks like GitHub actions for Windows have this conversion enabled by default. People in that ticket also mention issues with testing, so given that this problem is common for tests, disabling automatic line endings conversion can be a way to go.

   If this is an issue in our case, I don't have strong opinion on where to fix it, in assert_diff or in the ci config via overriding the git settings.

2. In xrefcheck's output, do we have cr-s or not?

   I suspect we need them, as otherwise not all terminals / editors (in case the user re-directs the output to a file) will manage to display that properly?

   On the other side, cr-s should already be included because TIL in Haskell's base package input/output streams are automatically updated to handle the newline endings 🤔

However judging by your symptops (as far as I got them correctly), I completely don't understand which of these points currently take place and which may need an adjustment.

[Sorokin-Anton]

@Martoon-00 finally, I've got two results:
If I gather xrefcheck's output via xrefcheck.exe > tmpfile, there are \rs
If I run xrefcheck.exe dump-config -t GitHub, thee are Unix line endings (no \rs)
I will try to use a different function so the config would be dumped with \rs.
About diffs - I'd prefer to use checkout with \rs since this would also allow us to simplify one Haskell test.

[Sorokin-Anton]

@Martoon-00 Bad news: if the repository was cloned with crlf's, output files have extra caret returns e.g. === Invalid references found ===\r\r\n\r\r\n
So I need some time to handle this

[Sorokin-Anton]

The reason of this is

ghci> TL.unlines ["a","b"]        
"a\nb\n"

and

ghci> TL.lines "a\r\nb\r\nc"
["a\r","b\r","c"]

while nyan-interpolation produced strings have \r's when they are in source file.
AFAIU in code we should think that no \r exists until printing anything.
I think the best way is to change nyan-interpolation code so interpolated strings never have \r's
Because if we have writeFile "aaaaaaaaaa" "a\nb\r\nc" (so in some places there was caret return and in some there were not), all \n s are transformed to \r\n during IO and those \r\r\n monsters appear

[Martoon-00]

Awesome. Very nice investigation!

I agree with your conclusions. Quite sad that this was produced by nyan-interpolation (suddenly, initially I would never suspect it). We will have to get it fixed there.

[Sorokin-Anton]

@Martoon-00 interesting fact: to_temp from bats always creates a file with LF endings 😐
I can't see why, looks like it is not a problem with our helpers. Same problem with assert_output
Looks like it's impossible to run assert_output --partial on multiline output on WIndows (and we use this to check error message in some cases).
So I'm leaving --strip-trailing-cr in assert_diff and rewriting those checks using multiple assert_output --partial.
Maybe one should create an assert_stderr_diff function later

[Martoon-00]

Meeh, I see.

Your solution to this sounds good. I have no other concerns.

---

Just a few words on this farewell I guess: that was a pleasure to review the work of yours. In my feeling, it required fewer review iterations than on average as you did everything mostly perfectly from the beginning. It was comfortable to me how initiative you were and has far you were ready to go when new issues arose. And concerns with English were pretty minor IMO.