Using Maybe instead of "UNKNOWN"?

[Blaisorblade]

I like this package a lot, but using "UNKNOWN" instead of Nothing means a user has to compare strings to handle this case specially. Would a variant returning Maybe be acceptable instead?

[acfoltzer]

How do you think this should look, concretely? One of the big advantages to the string-based approach is the simplicity of the TH splices, but I'm happy to consider other ideas.

[Blaisorblade]

I was thinking of a primary API using Maybe String instead of String, where UNKNOWN is possible, and exposing wrappers that do the last step. For maximum compatibility, the wrappers could expose the same API as they do now. Here's one untested sketch for gitHash: this still provides gitHash with the same behavior but also provides gitHashMaybe that you can splice to get a Maybe String, either Just theActualHash or Nothing.

runGit :: [String] -> IndexUsed -> Q (Maybe String)
runGit args useIdx = do
-- Adapted implementation omitted—change: use Nothing instead of `def`.

-- XXX Can't pick a good name
defaultStringE :: String -> Maybe String -> ExpQ
defaultStringE def mStr = stringE $ maybe def id mStr

-- | Reify a Maybe String.
--
-- XXX No TH expert, do I need to write this by hand?
maybeStringE :: Maybe String -> ExpQ
maybeStringE Nothing = [| Nothing |]
maybeStringE (Just t) = [| Just $(stringE t) |]

gitHashBase :: Q (Maybe String)
gitHashBase =
  runGit ["rev-parse", "HEAD"] IdxNotUsed

gitHashMaybe :: ExpQ -- Containing a Maybe String
gitHashMaybe =
  maybeStringE =<< gitHashBase

gitHash :: ExpQ -- Containing a String
gitHash =
  defaultStringE "UNKNOWN" =<< gitHashBase

This would apply where UNKNOWN can be returned, that is, as of https://github.com/acfoltzer/gitrev/blob/450a7abf444260e95791649489705e54650c0232/src/Development/GitRev.hs, in gitHash, gitBranch, gitDescribe, gitCommitCount and gitCommitDate.

I'm not happy about the extra boilerplate for each command but I'm not sure it can be much better (yes, I can abstract a bit more away, not sure it's worth it or it's much better). Probably OK?

[brandon-leapyear]

+1. I think having both options would be good, for a user who just wants a string (even if it's "UNKNOWN") and for a user that actually wants to check. It can be something like

runGit :: [String] -> IndexUsed -> Q (Maybe String)

runGit' :: [String] -> IndexUsed -> ExpQ
runGit' args = lift . runGit args

fromMaybeE :: String -> ExpQ -> ExpQ
fromMaybeE def (ConE ''Nothing) = stringE def
fromMaybeE _ (AppE _ s) = s

gitHash' :: ExpQ -- Maybe String
gitHash' = runGit' ["rev-parse", "HEAD"] IdxNotUsed

gitHash :: ExpQ -- String
gitHash = fromMaybeE "UNKNOWN" gitHash'