[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[TSVN] Re: RFC: Proposal for changes to patches

From: Simon Large <slarge_at_blazepoint.co.uk>
Date: 2004-10-07 19:54:44 CEST

"SteveKing" wrote:
> As soon as I read that, I _knew_ it would mean a lot of work coming my
> way ;)

I wouldn't want you to get bored ;)

> > The patch is created in the usual way using unified diff, then post
> > processed to strip out the files the user has not checked. Patch files
are
> > normally quite small, so this should not require a lot of processing.
>
> You mean it wouldn't require a lot of time for the processing -
> implementing it is a lot more work.

I may be able to help out if I can test the function from a console program.

> > Anything that SVN does not support should result in an error dialog
> > something like this:
> >
> > -------
> > Test2.c - added
> > Test3.c - deleted
> > -------
> > These actions are not currently supported in patch files and will not be
> > included. Do you want to create the patch file anyway? [Yes] [No]
> > -------
>
> Keep in mind though that a patchfile _can_ add files, and also (kind of)
> remove files. An added file is simply created, and a removed file is
> left with zero bytes length.

I would say that neither of those works. If I add a file, I want something
to be in it; if I delete a file I want it to go away.

> What definitely won't work is moving/renaming - the patchfile doesn't
> know that.

Which is why the user should be told which changes are not going to make it
into the patchfile.

> > The supplied log message should have an additional line inserted at the
> > beginning in the form:
> > [PATCH] Patchfile.ext yyyy/mm/dd hh:mm:ss
> > The time shown is the timestamp of the patchfile
>
> Why do you want the timestamp of the patchfile? Why is that important?

See below ...

> > Buttons here are
> > [Open] select a different patch file
> > [Apply]
> > [Cancel]
> >
> > If the user clicks on Apply, store the log message in the top level of
the
> > commit dialog's log history, then strip the log message from (a copy of)
the
> > patchfile and send to TMerge in the usual way.
>
> I think that TMerge should be the one to show the log message of the
> patchfile (it already can parse it, so the filelist would be easier to
> implement than in TortoiseProc).

Agree with that, as long as TMerge can add the log comment to the history.

> > If the patch is accepted, the log message is available at commit time
from
> > the log message history. Prefixing the name/date of the file helps to
> > identify the correct log message from the history list if there have
been
> > several attempts at patching.
>
> I think it would be _very_ bad practice to apply several patches with
> logmessages without committing them in between.

That was not the intention. Suppose I send you a patch and you review it and
reject it. The comment may already have gone into the history by the time
you reject it. Then I send you 3 more attempts. By the time you come to
commit there are 4 log messages to choose from. Which one is the correct
one? That is why the datestamp is there. Maybe when the message is added it
could check the history and delete any existing message for the same
patchfile. Then you would not need a stamp.

> The problem with patchfiles is that there isn't a standard which can
> handle binary diffs, properties and renames/moves (yet). The patchfile
> as it is used now was developed for CVS and hasn't changed since.

No, it's an improvement, but not a complete solution.

Simon

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org
Received on Thu Oct 7 21:03:57 2004

This is an archived mail posted to the TortoiseSVN Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.