Add stream option

[flsl0]

Hello,

I have added a new option that allows a process to stream its output to a specified source.

Basically I just duplicated the Output option and passed the given function to the OutputDataReceived and ErrorDataReceived callbacks.

[CaptnCodr]

Is it possible to have a new case (Stream) in Output?
The user would pass in an active stream where Fli writes to. That would minimize some redundancies.
What do you think about that or did you try that already?

[flsl0]

You are right, I haven't thought about directly passing a stream to the Output option.

Is this what you had in mind ?

[CaptnCodr]

Yes, that is what I meant. 🙂

One small thing, could you please add the Stream case in the README.md in section of Fli.Outputs here:

Fli/README.md

Lines 283 to 286 in 334b1bc

	Provided `Fli.Outputs`:
	- `File of string` a string with an absolute path of the output file.
	- `StringBuilder of StringBuilder` a StringBuilder which will be filled with the output text.
	- `Custom of Func<string, unit>` a custom function (`string -> unit`) that will be called with the output string (logging, printing etc.).

[flsl0]

I have updated the README, can you tell me if it's ok with you ?

Have you given some thoughts about my comments regarding the issue #63 you mentioned ? I have started a review but I don't know if you can see it on your end.

[CaptnCodr]

The README and your code looks beautiful. 👌🏻

The code doesn't build, that needs to be fixed.

I can't see any comments on my side from you. Could you list them in this discussion?
We can clear it out then.

[flsl0]

Thank you for the kind comment !

The build error seems to have come from the use of the new .Is* property : Discriminated union .Is* properties
Do you think it's possible to update the F# version used in the CI ? I could remove the latest commit if so.

My comments regarding the issue #63 went as follow :

For the following file src/Fli/Command.fs line 130 :
proc.OutputDataReceived.Add(fun args -> text <- text + args.Data ; outputFunc args.Data)
If I understand correctly to fully address the issue #63, this part text <- text + args.Data needs to be removed that way the process wouldn't be able to consume more memory than available but then the functions toText and printText in the module Output located in the file Output.fs wouldn't work as intended when streaming.

For the following file src/Fli/Command.fs line 131
proc.ErrorDataReceived.Add(fun args -> error <- error + args.Data ; outputFunc args.Data)
Following the same logic as above this part error <- error + args.Data should be removed as well but then the functions toError, printError and throw are impacted.

[CaptnCodr]

Do you think it's possible to update the F# version used in the CI ?

I would update the pipeline in a separated PR.

To your comments: these changes would change the whole library but using it with the stream option and relay the output to a file would minimize the mentioned memory issues in #63, isn't it?

[flsl0]

Oops, I kept mentioning the PR while referring to the issue #63 (fixed it in the earlier posts).

Yes exactly, with this change, redirecting the output to a file would ensure that the variable text (or error) is empty thus preventing it from growing as the process outputs.

Now that I think about it, maybe the user would expect the functions (toText, printText, toError...) to not work as intended (just to be clear they will still work but they print/manage an empty string) as he has explicitly redirected the output using the stream option.

It's also possible to create a new function that will take a string and the Output object as parameters and put the given string in Output.Text or Output.Error based on the Output.ExitCode (and wether or not Output.Text/Output.Error is empty) to create a well formatted Output object, like so :

let sb = StringBuilder()

cli {
    Shell CMD
    Command "echo Test"
    Output (new StringWriter(sb))
}
|> Command.execute
|> Output.From sb.ToString() // The new function
// the functions (`toText`,` `printText`, `toError`...) now work as intended

It's also possible to keep it as is or if you have any other ideas.

which one do you prefer ?

[CaptnCodr]

|> Output.From sb.ToString() // The new function

This solution looks beautiful, you can implement it. 🙂

[flsl0]

I have added the discussed method Output.from, can you tell me if it's ok with you ?

[CaptnCodr]

First sight looks good. I will look closer in the next few days.
Thank you for that beforehand.

[CaptnCodr]

Hey @flsl0,
your changes look good. I will merge your changes.
If you want to upgrade the project to net9.0 and update the pipeline and use F# 9, I create an issue for that.

[CaptnCodr]

Sorry for the delay, I have a lot to do in my life right now.