Does anybody object in principle? It seems useful to me, and doesn't seem very controversial, so I'll leave the question open for a couple of days before accepting this patch on my next round (which is regrettably delayed).
This patch outputs some status information about a repository, for
example:
Type: Darcs
Format: darcs-1.0
Root: /Users/kowey/darcsImpl/stable
Pristine: PlainPristine "_darcs/pristine"
Cache: thisrepo:/Users/kowey/darcsImpl/stable
predist Pref: autoconf && ./configure && ${MAKE-make} predist
test Pref: autoconf && ./configure && ${MAKE-make} check && ${MAKE-make}
Default Remote: http://www.darcs.net/repos/stable
# Patches: 4370
Information which is not available in the repository is not shown.
Comments for Kevin
------------------
I have some mainly stylistic suggestions, which I hope would make the
code easier to read. The patch would still go in anyway if there are
no suggestions, but if anybody wants to tidy up, here are some thoughts.
As a general note: Kevin, please set your text editor to show trailing
whitespace. No big deal, but I'd like to reduce it as much as possible.
> +The \verb!show repo! (or \verb!query repo!) displays information about
> +the current repository: the location, the type, etc.
Note: we should probably not encourage people to use the 'query' alias
> +\verb!darcs query repo | grep Root: | awk {print $2}!
Likewise.
> +show_repo_description = "Show repository information"
Maybe the word 'summary' would be useful somewhere
> +putInfoLn :: ShowInfo -> PutInfo
> +putInfoLn m t i = unless (null i) (putStr $ m t i)
The 'Ln' makes for a bit of a misnomer. When I see that, I'm expecting
a newline at the end.
> +-- labelled strings: labels are right-aligned at 14 characters;
> +-- subsequent lines in multi-line output are indented accordingly.
> +showInfoUsr :: ShowInfo
> +showInfoUsr t i = (replicate (14 - length(t)) ' ') ++ t ++ ": " ++
> + (concat $ intersperse ('\n' : (replicate 16 ' ')) $ lines
> i) ++ "\n"
First, I'm not so keen on the fancy right-aligning code. I realise that
it can be helpful for human readibility, but I wonder if we can find
something that's a little bit friendlier to future changes. For example,
maybe we could just use a tab character? The output is not quite as
nice-looking, but the underlying code is simpler. I guess it depends on
whether you're more interested in people or third party tools.
Second, how about simplifying that bottom bit to
(unlines . map pad . lines $ i)
where pad x = replicate 16 ' ' ++ x
Finally, I get the impression that you don't actually use this padding
functionality (you seem to use ', ' to fit everything on one line)
> +showRepo :: PutInfo -> Repository -> IO ()
> +showRepo out r@(Repo loc opts rf rt) =
> + showRepoType out rt
> + >> when (Verbose `elem` opts) (out "Show" $ show r)
> + >> showRepoFormat out rf
> + >> out "Root" loc
> + >> showRepoAux out rt
> + >> showRepoPrefs out
> + >> unless (NoFiles `elem` opts) (numPatches r >>= (out "# Patches"
> . show ))
> + >> showRepoMOTD out r
Since this feature can eventually be used by third party tools, you may
consider simplifying their life a tiny bit by using "Num Patches"
instead of "# Patches".
Also, in the spirit of keeping things simple, you can probably just not
capitalise the tags. This avoids you having to jump through the hoops
of decapitalising them later on, for example, in the XML code. Getting
rid of the '#' and the capitalisation can bring us down to something
like safeTag = filter (not . isSpace)
Finally, I think this is a good fit for do notation :-) I know you're
not a big fan, but do-notation is also more darcs-like. Hooray for
mindless conformism.
> +showRepoType :: PutInfo -> RepoType -> IO ()
> +showRepoType out GitRepository = out "Type" "GIT"
> +showRepoType out (DarcsRepository _ _) = out "Type" "Darcs"
Well, here you seem to want to pay attention to capitalisation. In that case,
I would suggest "Git" and "darcs" which is what both projects seem to use.
> +showRepoPrefs :: PutInfo -> IO ()
> +showRepoPrefs out =
> + get_preflist "prefs" >>= mapM_ (uncurry out . (\(p,v) -> (p++" Pref",v))
> . break (==' '))
> + >> get_preflist "author" >>= out "Author" . unlines
> + >> get_preflist "defaultrepo" >>= out "Default Remote" . unlines
Here's how I might have rewritten this
showRepoPrefs :: PutInfo -> IO ()
showRepoPrefs out =
do get_preflist "prefs" >>= mapM_ prefOut
get_preflist "author" >>= out "Author" . unlines
get_preflist "defaultrepo" >>= out "Default Remote" . unlines
where
prefOut pv = case break isSpace pv of
(p,v) -> out (p ++ " Pref") (dropWhile isSpace v)
Also, break seems to include the space, which gives
Cache: thisrepo:/Users/kowey/darcsImpl/stable
predist Pref: autoconf && ./configure && ${MAKE-make} predist
test Pref: autoconf && ./configure && ${MAKE-make} check && ${MAKE-make}
The above code gets rid of that space.
> +showRepoMOTD :: PutInfo -> Repository -> IO ()
> +showRepoMOTD out (Repo loc opts _ _) =
> + fetchFilePS opts (loc++"/_darcs/prefs/motd") (MaxAge 600)
> `catchall` return nilPS
> + >>= out "MOTD" . unpackPS
You might consider refactoring this with Darcs.Repository.Motd
--
Eric Kow http://www.loria.fr/~kow
PGP Key ID: 08AC04F9 Merci de corriger mon français.
pgpU99R2FUA8p.pgp
Description: PGP signature
_______________________________________________ darcs-devel mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-devel
