Hmm... I'm a bit hesitant. See below. On Sun, Feb 01, 2009 at 18:50:30 -0800, Trent W. Buck wrote: > I'm sick of this sitting on my hard disk, and I want it applied to > trunk. So I resolved conflicts with trunk and am sending it. > > Mon Feb 2 13:41:46 EST 2009 Trent W. Buck <[email protected]> > * Resolve issue1093: warn about ugly patch names. > > Mon Feb 2 13:51:14 EST 2009 Trent W. Buck <[email protected]> > * Test issue1093: warn about ugly patch names. >
Resolve issue1093: warn about ugly patch names.
-----------------------------------------------
> +-- FIXME: should print *all* matching style warnings.
Not that you were asking for suggestions on how to go about doing this,
but I wonder if this would be a suitable approach
diagnostic match w n = if match n then Just w else Nothing
diagnostics =
[ diagnostic (\n -> length n < 10) "WARNING: ..."
, diagnostic (\n -> length n > 70) "WARNING: ..."
]
mapMaybe ($ n) diagnostics
> +check_name_is_not_ugly :: [DarcsFlag] -> IO ()
> +check_name_is_not_ugly opts = when (length patchNames == 1) $ do diagnose
> (head patchNames)
> + where patchNames = [n | PatchName n <- opts]
> + diagnose :: String -> IO ()
> + diagnose n@('-':_) =
> + putStrLn ("WARNING: the patch name \"" ++ n ++ "\" looks like
> an option.") >>
> + putStrLn "This is discouraged because it may confuse
> command-line utilities." >>
> + amend
> + diagnose n | length n < 10 = -- this number is totally arbitrary
> + putStrLn "WARNING: the patch name is very short." >>
> + putStrLn "Does it convey an adequate summary to
> other contributors?" >>
> + amend
If we were to prompt to amend (I don't think we should by the way, but
that's just a gut feeling, best to observe users to find out), this
could become very annoying very fast. I love that darcs is friendly and
helpful, but I am also a bit afraid of crossing the line into being
officious. Can we find a way to help while staying politely out of the
way?
> + -- Note that we use isLower here instead of not
> + -- . isUpper because some languages are caseless.
> + | isLower $ head n =
Your comment is interesting. So I presume this means that in caseless
languages, isUpper is always True and isLower is always False? Also,
this use of head could explode on us, as I don't see us having trapped
the null case anywhere. Seems that we could do this just as well with
a pattern match
> + | not $ isPunctuation $ last n =
> + putStrLn "WARNING: the patch name doesn't end with
> punctuation." >>
> + putStrLn "Using a whole sentence for each patch
> name is recommended." >>
Again, kaboom warning (last) and also imposing-our-preferences-on-others
warning. It's one thing if we're catching patch names that start with
'-' because, very likely, the user did not intend to write such a patch
name. It's quite another thing if we start telling users how they
should want to write patch names.
Test issue1093: warn about ugly patch names.
--------------------------------------------
Thanks
--
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
pgpW5Q59C8a6N.pgp
Description: PGP signature
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
