PS color output fix

[Vanello1908]

Fixed one of bugs mentioned in this issue

[daveyarwood]

Hi, thanks for the PR! I tried this branch locally and unfortunately, it doesn't compile on Linux. I get this error:

$ go build .
package alda.io/client
        imports alda.io/client/cmd
        imports alda.io/client/color
        imports golang.org/x/sys/windows/registry: build constraints exclude all Go files in /home/dave/.go/pkg/mod/golang.org/x/[email protected]/windows/registry

I did some googling and found this StackOverflow question which looks like the same issue.

If we wanted to get this approach to work, I think we would need to use build constraints to exclude the Windows registry code when the code is being compiled and run in a non-Windows environment.

I did a little test of this by copy-pasting the code you added into a new file called color_windows.go, and it did build successfully after that. However, it won't exactly work that way, because there's a potential race condition where we could end up running Aurora = auroraLib.NewAurora(EnableColor) in color.go before the code in color_windows.go sets EnableColor appropriately.

If you'd like to keep working on this approach and adjust it along those lines so that it will compile on Linux, you're welcome to do so. You can check the CircleCI logs to see if the build succeeded, i.e. click the ❌ or ✔️ next to the commit message:

And then click the "Details" link to go to the build logs in CircleCI:

[daveyarwood]

My other thought on this is that it would be better if we didn't have to include a bunch of ad hoc code in Alda to work around OS-specific issues. We should really let the color library handle this for us.

The version of the color library we're using (Aurora) is from 2020, which is several years old at this point. I think it's worth trying to update to the latest version and see if it works better out of the box in Powershell.

If that doesn't work, switching to another color library is also an option. @KenP97 took a stab at that in #420, using the Gookit library.

[Vanello1908]

Ok, this is done with build constraint. Next step - black symbols, it is a problem with readline lib. There are 3 options:

• create pull request to that lib with fix
• create fork with fix
• use another lib for input
  But I want not to use the third option

[daveyarwood]

I think you can remove this line. The Go compiler recognizes when the file name ends with _windows.go and only includes it in the build if the OS is Windows.

(You also have a typo in the filename -- should be color_windows.go)

[daveyarwood]

Let's put this comment back, as it's still relevant.

[daveyarwood]

We probably don't need this check, as this file will only be included in the Windows build.

[daveyarwood]

Looking good so far! If you don't mind implementing my suggestions above, I think this will be ready to merge.

I am curious if you've tried updating the Aurora dependency, to see if maybe the library has improved in the last 4 years to the point that none of this OS-specific code is necessary. But I'm also fine with the approach in this PR, at least as an interim improvement.

[Vanello1908]

Aurora v2.0.3+incompatible also has problems with PowerShell, so we remain this code

[daveyarwood]

Looks good! I'm going to go ahead and merge this and cut a patch release of Alda. I appreciate your work on this, and will be happy to review any future PRs to address the remaining Windows color issues.