ianmackenzie / elm-script

Experimental command-line scripting for Elm
34 stars 4 forks source link

Script.andThen is misleading #17

Open MartinSStewart opened 4 years ago

MartinSStewart commented 4 years ago

I tried using Script.andThen like this

Script.File.readOnly init.userPrivileges path
     |> Script.File.read
     |> Script.andThen (\text -> Script.succeed text)

I was expecting it to work the same way andThen works in most parser/decoder packages. Finally I realized that I need to use thenWith instead. Maybe andThen and thenWith could swap names?

ianmackenzie commented 4 years ago

I spent a lot of time struggling with this, and it could be there's a nicer solution somewhere (although a simple note in the docs might also go a long way). What I found from writing a bunch of scripts was that it seemed pretty common to sequence two scripts together where the second didn't care about the result of the first - see for example this function for generating an Elm file with a bunch of GLSL shaders. With a standard andThen, you'd end up with a lot of things like

Script.printLine ("Writing " ++ File.name outputFile ++ "...")
        |> Script.andThen (\() -> File.write contents outputFile)
        |> Script.andThen (\() -> Script.printLine "Formatting output file with elm-format...")

which is a bit cluttered, and potentially error prone since if you write it like

Script.printLine ("Writing " ++ File.name outputFile ++ "...")
        |> Script.andThen (\_ -> File.write contents outputFile)
        |> Script.andThen (\_ -> Script.printLine "Formatting output file with elm-format...")

then you could end up accidentally ignoring the output of a previous script (the current andThen has some nice type safety in that you can only use it after a script that has no meaningful output, i.e. a result type of ().)

I really like how thenWith reads, but I admit it will definitely be confusing for people - I could see renaming thenWith to andThen, but I don't think you could swap the two since thenWith would no longer make sense. (thenWith what? The whole point of that function is that it's for cases where the previous script doesn't produce any output to do anything with...)

An alternative could be something like just then_, which is kind of ugly but at least short, and arguably gets across the idea that it's a simpler version of andThen.

On the other hand, thenWith does extend more naturally to things like thenWithResult as proposed in #18...it could be that inherently sequential, effectful code is different enough from 'normal' Elm code that it warrants having some slightly different patterns.

MartinSStewart commented 4 years ago

I think andThen should still have thenWiths type signature. elm-script is conceptually similar to Task, so I think it's important that there aren't any functions with the same name but subtly different behavior and type signature. I think Script.attempt, for example, is okay because the difference between it and Task.attempt is more noticeable.

I admit I don't have any good suggestions for alternative names. then_ might be the best alternative for andThen (though I agree, it's a bit ugly).

One thought though, is andThen necessary? If the scripts don't depend on each others results, maybe it would be easier to use do [ firstThing, secondThing, thirdThing ]?

ianmackenzie commented 4 years ago

One thing do doesn't allow for is returning a result from the final script in a chain. For example, here's some code I wrote to do some GLSL minification:

Script.printLine ("Optimizing " ++ Glsl.shaderName givenShader)
    |> Script.andThen (File.writeTo inputFile (Glsl.shaderSource givenShader))
    |> Script.andThen
        (Script.executeWith userPrivileges
            { command = "glsl-minifier.cmd"
            , arguments =
                [ "-i"
                , File.name inputFile
                , "-o"
                , File.name outputFile
                , "-sT"
                , shaderTypeString
                , "-sV"
                , "2" -- WebGL 1
                ]
            , workingDirectory = tempDirectory
            }
            |> Script.ignoreResult
        )
    |> Script.andThen (File.read outputFile)
    |> Script.map fixSource
    |> Script.thenWith
        (\optimizedSource ->
            Script.succeed (givenShader |> Glsl.setShaderSource optimizedSource)
        )

Chaining with the current implementation of andThen only requires the previous script to have a () return type; you can use it to chain into a script which does have a return value, which is used here to read from a file and do some postprocessing on the contents before returning a final result.

I'm thinking of renaming andThen -> thenDo and thenWith -> andThen; looking at a few example scripts I think that reads reasonably well. For example, the above code would turn into

Script.printLine ("Optimizing " ++ Glsl.shaderName givenShader)
    |> Script.thenDo (File.writeTo inputFile (Glsl.shaderSource givenShader))
    |> Script.thenDo
        (Script.executeWith userPrivileges
            { command = "glsl-minifier.cmd"
            , arguments =
                [ "-i"
                , File.name inputFile
                , "-o"
                , File.name outputFile
                , "-sT"
                , shaderTypeString
                , "-sV"
                , "2" -- WebGL 1
                ]
            , workingDirectory = tempDirectory
            }
            |> Script.ignoreResult
        )
    |> Script.thenDo (File.read outputFile)
    |> Script.map fixSource
    |> Script.andThen
        (\optimizedSource ->
            Script.succeed (givenShader |> Glsl.setShaderSource optimizedSource)
        )

Another option would be to rename andThen to andThenDo, and thenWith to andThenWith; that way at least the function names aren't quite as misleading as currently. And the docs can explain why there are two versions (in normal Elm it never makes sense to ignore the result of the previous value, but when scripting it often does). Thoughts?

MartinSStewart commented 4 years ago

One thing do doesn't allow for is returning a result from the final script in a chain.

Good point. I prefer this one andThen -> thenDo and thenWith -> andThen then.