[darcs-users] darcs patch: Add optional specifier for "darcs help environment [VA...

Eric Kow kowey at darcs.net
Mon Oct 5 10:53:39 UTC 2009


On Sat, Oct 03, 2009 at 18:30:33 +1000, Trent W.Buck wrote:
> This has been sitting in my repo for some time, because I've been too
> embarrassed to send it.  The code is fugly, and I'm not convinced that
> the functionality is desirable.

Some suggestions to consider.

Add optional specifier for "darcs help environment [VARIABLE]".
---------------------------------------------------------------
> -help_cmd _ ["environment"] = viewDoc $ text $ helpOnEnvironment
> +help_cmd _ ["environment"] = viewDoc $
> +                             text "Environment Variables" $$
> +                             text "=====================\n" $$
> +                             vsep [text (andClauses ks ++ ":") $$
> +                                   vcat [text $ "  " ++ d | d <- ds]
> +                                   | (ks, ds) <- environmentHelp]

Just a Trent-style refactor as far as I can tell (kill helpers that are
only used once), as well as one to use printer functions instead.  You
may be happier with some more printer helpers, such as (<+>).

Also, I find the embedded list comprehensions a little difficult to
read.

How about

                             vsep [text (andClauses ks ++ ":") $$ vcat (map (space <>) ds)
                                  | (ks, ds) <- environmentHelp]

> +help_cmd _ ["environment", var] = viewDoc $ render $ entry environmentHelp
> +    where render (ks, ds) = text (andClauses ks ++ ":") $$
> +                            vcat [text $ "  " ++ d | d <- ds]

Sounds like you have a bit of code duplication here with the above.  I
think could could consider

help_cmd _ ("environment" : vars) =
  case vars of
    [] -> ...
    _  -> ...
  where
    render ...

Which also means you'd (somewhat uselessly) account for darcs help
environment DARCS_EMAIL DARCS_AUTHOR

> +          entry [] = error $ "undocumented environment variable: " ++ var
> +          entry (x:xs) | map toUpper var `elem` fst x = x
> +                       | otherwise = entry xs

Error should only be used for the impossible.

This sounds more like a user error, which we should (perhaps) percolate,
trap and report.  I think Darcs.Utils may have pretty exceptions that we
could use for that.

Otherwise, this looks like a filter.  Perhaps something like?

 case filter (hasVar v) environmentHelp of
   [] -> complain! I don't know what FOO is
   xs -> display help on xs...

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20091005/7937c946/attachment.pgp>


More information about the darcs-users mailing list