[darcs-devel] Greetings and some questions

[David Roundy]

On Mon, Oct 02, 2006 at 10:39:36AM -0700, Jason Dagit wrote:
> On 10/2/06, Dino Morelli <dino at ui3.info> wrote:
> >Also, I see a lot of underscore-style functions like
> >get_long_darcs_opt_string. Is there a naming preference for this project?
> 
> I can only speak for myself, but I'd say no underscores.  I prefer
> camel case for Haskell code.  Others?

I traditionally prefer underscores, but camel case seems to be more common
in the Haskell world, and darcs is moving in that direction.  My current
leaning is to use camel case for exported functions, and underscores (which
I do find easier to read) for internal functions.  And I like to keep
modules consisted in this respect, so older modules should remain
all_underscore, unless someone wants to refactor them (which doesn't seem
worth the effort).

Oh, and DataTypes (or Classes... anything Haskell has starting with a
capital letter definitely should be camel case).

> >Working on implementation, I needed to import to get this function for
> >DarcsArguments:
> >
> >    import DarcsCommands ( get_long_option )
> >
> >
> >And I'm getting an error:
> >
> >    Module imports form a cycle for modules:
> 
> Yes, I've started getting this in some cases too.  There are a couple
> ways to solve it and all of them are usually orthogonal to what you're
> really doing (meaning it's slightly annoying).  One trick is to use
> hs-boot files as per the GHC manual.  Doing this with our current make
> file framework is a pain.  The other common solution is to adjust the
> modules so that you can import things in a way that doesn't cause a
> cycle.  A cycle basically means at least two modules directly depend
> on each other.

Usually this means that we've got some mixed up (and human-unfriendly) code
organization going on, and the code will benefit in readability from
adjusting the modules.  In this case, get_long_option really belongs in
DarcsArguments, which defines the relevant data types.  I also notice, that
get_long_option seems to be buggy in its treatment of options that have
more than one synonymous long option.  I'm not sure that we use any such
options, but it looks to me like get_long_option is potentially buggy.
This is another danger of "blindly" exporting functions that aren't
currently exported at all: they might be bugless in their current use, but
misbehave under different usages.  Ideally, exported functions shouldn't
behave like this, since they should be designed to be resistant to misuse
(either in that their name unambiguously defines their meaning, or even
better if their type can unambiguously define the meaning).

wrt the idea of extracting flag names from their DarcsFlag, I think I'm
probably with Eric on this, in that it seems like a good idea.  I can see
Tommy's point that in general it's a bug to change the name of any flags,
but making things "automatically right" is just a good thing.  You could
even consider a function like

explain_flag :: DarcsOption -> String

which would invoke a "canned" explanation of what the flag does, which
might allow us to create "expanded" help for the manual (or separate man
pages for each darcs command?).  But probably this would be overkill.
-- 
David Roundy
http://www.darcs.net