Hannes (Jan 17 2024 at 05:31): Hannes (Jan 17 2024 at 05:31):I wrote my own fortune script in Python a while ago, and I just rewrote it in Roc to get a comparison. I'm interested to get some feedback on if what I've written is "idiomatic Roc", plus there's a bug where the command always produces an exit code of 1, so if someone has a suggestion for what I'm doing wrong that'd be good too. The repo is here. Thanks in advance :)
Brendan Hansknecht (Jan 17 2024 at 05:41):List.keepIf (\f -> f |> Str.isEmpty |> Bool.not)
List.dropIf Str.isEmpty
Brendan Hansknecht (Jan 17 2024 at 05:43):Nit, skip the variable here, just leave is as one pipe:
fortunes = fortunesStr |> ...
getRandomFortune fortunes
Just:
fortuesStr
|> ...
|> getRandomFortune
Stderr.line and Stdout.line will never error:
Stderr.line "Couldn't get config folder :(" |> Task.mapErr (\_ -> 1)
You probably want to print the message then set the error value no matter what. So that would be an await followed by Task.err 1
Brendan Hansknecht (Jan 17 2024 at 05:50):pathAppend : Path.Path, Str -> Path.Path
pathAppend = \p, s -> p |> Path.display |> Str.concat "/" |> Str.concat s |> Path.fromStr
.... I guess we missed that api in Path. Definitely should add that to basic cli.
Brendan Hansknecht (Jan 17 2024 at 05:53):Personally I am not a big fan of the long line piping:
fortunes |> List.get randomIndex |> Result.withDefault "" |> Task.ok
I think it reads nicer as:
fortunes
|> List.get randomIndex
|> Result.withDefault ""
|> Task.ok
Why remove your most random values with division?
|> Utc.toMillisSinceEpoch
|> Num.divTrunc 1_000_000_000
Also, why not Nanos?
Brendan Hansknecht (Jan 17 2024 at 05:56):Personally I find that this reads quite dense:
getXdgConfigHome = Env.var "XDG_CONFIG_HOME" |> Task.map Path.fromStr
getHome = Env.var "HOME" |> Task.map Path.fromStr
getXdgConfigHome
|> Task.onErr (\_ -> getHome |> Task.map (\h -> h |> pathAppend ".config"))
|> Task.mapErr (\_ -> CouldntGetConfigFolder)
Especially this line:
|> Task.onErr (\_ -> getHome |> Task.map (\h -> h |> pathAppend ".config"))
No immediate fix thought, but not a fan of it.
Brendan Hansknecht (Jan 17 2024 at 05:57):Ok. That is my list of essentially all nit picks
Brendan Hansknecht (Jan 17 2024 at 06:00):As for returned exit code, it looks like basic cli currently ignores the exit code. We used to have Process.exit, at least I think we did, but that also seems to be gone.
Brendan Hansknecht (Jan 17 2024 at 06:00):So the exit code being wrong is a basic cli bug
Hannes (Jan 17 2024 at 06:09):Thanks Brendan, that's really helpful! Lots to improve :)
The reason I divided by 1 billion is because the value returned by toMillisSinceEpoch always ended with many zeros, not sure why :shrug: I'll look into it when I do the other refactors you've suggested
Glad to know the exit code isn't a problem with my code :)
Brendan Hansknecht (Jan 17 2024 at 06:16):https://github.com/roc-lang/basic-cli/pull/158
Brendan Hansknecht (Jan 17 2024 at 06:21):The reason I divided by 1 billion is because the value returned by toMillisSinceEpoch always ended with many zeros, not sure why :shrug: I'll look into it when I do the other refactors you've suggested
Another bug in basic cli
toMillisSinceEpoch : Utc -> U128
toMillisSinceEpoch = \@Utc nanos ->
nanos * nanosPerMilli
That should be division
Brendan Hansknecht (Jan 17 2024 at 06:23):https://github.com/roc-lang/basic-cli/pull/159
Luke Boswell (Jan 17 2024 at 10:18):Nice work fixing those.
Last updated: Jul 06 2025 at 12:14 UTC