[darcs-devel] darcs patch: allow submitting patches via HTTP POST in addition to ...

[David Roundy]

On Mon, Mar 31, 2008 at 09:10:38PM -0700, John Meacham wrote:
> On Mon, Mar 31, 2008 at 07:08:36AM -0700, David Roundy wrote:
> > On Sat, Mar 29, 2008 at 08:46:27PM -0700, John Meacham wrote:
> > > Sat Mar 29 20:44:51 PDT 2008  John Meacham <john at repetae.net>
> > >   * allow submitting patches via HTTP POST in addition to email.
> > 
> > Applied, thanks! Could you add "send --unified" to your _darcs/defaults,
> > please? It'd make it easier for me to review your patches.
> 
> Ah, I didn't even know about that feature. I think I am going to start
> asking people submitting patches to jhc to use that, is there any way to
> specify that as a default on the server side? I manage quite a few
> projects with darcs and that would certainly help with evaluating
> patches for projects I have paged out of my brain. :)

No, we don't have a server-side solution for this.  It'd be nice, but I'm
not quite sure how we'd manage it in a pretty-to-code way.  We certainly
don't want to allow server-side specification of default flags (as that'd
be a security risk--just think of --posthook... of course, if you're
downloading code from a server, *usually* you're already trusting that
server to execute arbitrary code on your machine, but not always).  We
could add a configuration that is send --unified-specific, but that's a bit
ugly.

Another option (which someone just suggested) would be to just make
--unified the default for send.  That alone isn't a possibility, since
currently there's no inverse flag for --unified.  So either we'd have to
add that flag, or simply make darcs send always output --unified patches.
They're a bit larger than the default patches, and a bit more expensive to
compute.  But normally neither of those are significant... probably not
even for something huge like the ghc repository.  So maybe just removing
the flag and making send always send unified patches would be the nicest
approach.  The idea of simplifying the interface certainly appeals.

> Actually, one of my motivations for this patch was a server script that
> automatically entered incoming patches into a bug database and spit out
> a url. so when the contributer does a darcs send they get a url to a
> tracking page back where they can check on the status of their patch and
> not have to worry if it was eaten by a spam filter or I just forgot
> about it or something.

Sounds nice.

> Also, I was curious if you would be interested in some larger code
> cleanup work on darcs? Working on Send I found a lot of repeated code
> that I think could be wrapped up nicely in a monad that will pipe darcs
> environment like the flags around. Would you mind if I worked on this? I
> think it will make it easier to contribute in the future.

That's an offer that's hard to turn down, but I'm actually pretty fond of
the idea of passing around those flags, simply because I like making
explicit the distinction between functions that depend on the user flags
and those that always do the same thing.  When the flags are passed
manually, then -Wall -Werr ensures that functions that are unnecessarily
passed the opts are obvious, because they've got a glaring "_" in their
argument list.

I have a fear that if you moved functions into a monad, we'd end up with
functions creeping into that monad and never leaving.

I've not been entirely consistent, however, since the Repository data type
hides within it a copy of the flags.

I'd like to keep as much of darcs as possible written in "simple" Haskell,
so it's easy for folks who are just learning Haskell (which isn't entirely
consistent with the fact that I've been rewriting the core of xmonad to use
gadt type witnesses--but that has a very large safety benefit, which ought
to make it far safer for folks who don't fully understand the patch theory
to contribute).  Speaking of which, that's another factor:  we've got a
major (probably multi-year) refactoring underway, which is the addition of
type witnesses.  We'd have to be cautious that the two refactorings didn't
interfere with each other.  Probably this wouldn't be a problem, since your
work would be from the bottom up, and the type witnesses are migrating from
the top down (well, depending which way you orient the code).

Perhaps you could propose a monad, and send it in a separate email with a
subject that will lure other folks to read it and give opinions? The
most important question is whether it'll make it easier (or harder) for
*other* folks to contribute to darcs.

Oh, and here's counter-proposal.  

I actually really dislike how we pass [DarcsFlag] all over the place, as
it's a very un-typed approach.  You can't tell what information you're
passing in, or why, or what effect it will have.  In some cases, this is
desirable.  It allows us, for instance, to write a command that accepts
--dont-compress without writing any code in that command that understands
or interprets that flag in any way, which aids in enabling consistent
interpretation of flags (good for the user).

What if we make a move to eliminate [DarcsFlag] where-ever possible,
replacing it with custom ADTs defined in Darcs.Arguments?  e.g. so
functions that just want to know the Verbose/Quiet flags would accept a

data Verbosity = IsVerbose | IsNormal | IsQuiet

and functions that call these functions would use a helper function (also
defined in Darcs.Arguments) called

getVerbosity :: [DarcsFlag] -> Verbosity

This couldn't (shouldn't?) eliminate all functions (outside of
Darcs.Arguments) that accept [DarcsFlag], but it might eliminate most of
them, and could make it much clearer what information is being passed,
while still maintaining some level of insulation, in that the commands
wouldn't need to actually know which particular flags are involved.

I'm not convinced to what extent this would actually improve the situation,
but it might be that with just a few ADTs one could capture most of the
situations where we're passing [DarcsFlag] around, and make the code a
little bit less spaghetti-like at the same time (in the sense that you
wouldn't need to delve all the way down into Darcs.External or URL or Ssh
in order to know which flags are used by a given function.
-- 
David Roundy
Department of Physics
Oregon State University