Patch for preliminary Mercurial integration in kdevplatform

[Fabian Wiesel]

On Fri, 13 Mar 2009 22:14:50 +0100
Andreas Pakulat <apaku at gmx.de> wrote:

> Thats for example wrong. If rawOutput contains non-ascii text this
> will break horribly. 

I admit, I haven't thought about non-single-charsets. Fixed that in the
new version (now submitted over commited).

Speaking about multi-byte character sets, doesn't the approach in
ProcessLineMaker has its own problems?
First one, what happens, when the output is not terminated by a final
newline? As it currently stands, the final will not be passed by
output().

Second, do we care about about 2-byte (or four-byte) character sets?

> [...]
> Just curious whats the point of using std::auto_ptr here?
> > +DVCSjob* MercurialExecutor::init(const KUrl &directory)
> > +{
> > +    std::auto_ptr<DVCSjob> job(new DVCSjob(vcsplugin));
> > +
> > +    if (!prepareJob(job.get(), directory.toLocalFile(),
> > MercurialExecutor::Init)) {
> > +        return NULL;
> > +    }
> > +
> > +    *job << "hg" << "init";
> > +
> > +    return job.release();
> 
> Or especially here as you're giving it away anyway.

I like RAII, and have an aversion of unowned pointers.
As I gathered from the code DVCSPlugin, a job has to be deleted unless
it is started. Then the job itself will take ownership. When I return
the pointer, I will relinquish the ownership to the caller.
Should prepareJob() (or anything else before returning the pointer fail,
I have to delete the pointer.
I'd rather do that by an RAII-object than manually, especially since it
is practically the same amount of code. Doing so even in a three-liner
is just a matter of consistency.

Of course, I can change that, if it goes against code-style here.


> Interesting, mercurial supplies \0 as separation character. Thats
> rather nice for stdin/stdout communication.

Well, mercurial doesn't do that by itself, but it can be motivated to
do so by providing a template how the output should look like.
Unfortunatly, that only works for the log-output.

> Other than that the code seems ok (I didn't do a really deep review
> as I don't know too much about the dvcs stuff). Are you willing to
> maintain this if it goes into kdevplatform? Do you already have an
> svn account?

Generally, yes. I could maintain it. I don't have an SVN account yet.

Fabian