fix(prqlc-python): fix the display option works

[eitsupi]

A follow up for #4333
So far the display option has not worked.

import prqlc
options1 = prqlc.CompileOptions(
    format=True, signature_comment=True, target="sql.postgres", display='plain'
)
options2 = prqlc.CompileOptions(
    format=True, signature_comment=True, target="sql.postgres", display='ansi_color'
)
prqlc.compile("from a | select")
prqlc.compile("from a | select", options=options1)
prqlc.compile("from a | select", options=options2)

By the way I do not know the best way to test this option.
IIUC asntream automatically detects the state of the output and decides whether to include the ansi code or not, so if we take a snapshot in the interactive development environment, the ansi code is included but on CI the ansi code is removed, so the snapshot test fails.

My understanding is that this behavior of the display option is not very ideal, and ideally the ansi code should always be included when 'ansi_color' is specified.

[eitsupi]

In my opinion, the display option should be mapped to anstream::ColorChoice.
https://docs.rs/anstream/latest/anstream/enum.ColorChoice.html

prqlc-r used to always set anstream::ColorChoice::Never to get rid of ansi codes.
https://github.com/PRQL/prqlc-r/blob/dde9f1bf2c416cdfd7d06458f51034701c671cd2/src/rust/src/lib.rs#L34

[max-sixty]

Much better code!

[max-sixty]

I agree the current state is not ideal. Though I have gone back & forth between a few options and haven't found one that's totally satisfying.

In my opinion, the display option should be mapped to anstream::ColorChoice.
https://docs.rs/anstream/latest/anstream/enum.ColorChoice.html

This is has merits. I was thinking that at some point we could enable more options, such as HTML. (Ansi color codes are not a great way of encoding color, we just use them because that's what terminals expect...)

By the way I do not know the best way to test this option.
IIUC asntream automatically detects the state of the output and decides whether to include the ansi code or not, so if we take a snapshot in the interactive development environment, the ansi code is included but on CI the ansi code is removed, so the snapshot test fails.

Yes, the testing is difficult with these.

Possible our default should be "Auto", which defers to Anstream? And if someone passes AnsiColor then it should always produce color codes? Then we could at least test the AnsiColor option...

[eitsupi]

Thanks!

This is has merits. I was thinking that at some point we could enable more options, such as HTML.

I see, that makes sense.

Possible our default should be "Auto", which defers to Anstream? And if someone passes AnsiColor then it should always produce color codes? Then we could at least test the AnsiColor option...

I wanted to use this in prqlr's knitr integration to output colored errors on the Quarto document, but now it seems impossible as is. (I may have to use the anstream::ColorChoice::write_global function for now)

[max-sixty]

I wanted to use this in prqlr's knitr integration to output colored errors on the Quarto document, but now it seems impossible as is. (I may have to use the anstream::ColorChoice::write_global function for now)

Would changing the behavior to Auto allow this?