On 10.03.2010 22:43, Stefan Sperling wrote:
On Wed, Mar 10, 2010 at 08:58:24PM +0100, Stefan Küng wrote:
On 10.03.2010 19:14, Julian Foad wrote:

Yes, I would also need to know which paths the patch wants to modify,
and then later I want to tell the API which of those paths it should
actually modify.
This is to let the user choose which paths to modify and which ones not.

OK, well the issue #3434 didn't mention that. Since you already have a
patch implementation which does what you want, could you show us the API
definition (at least) for this part of it? That would help a lot, since
then we wouldn't have to guess about what you want :-)

It's basically a class with methods like:
* ParsePatchfile
* GetCount //returns the number of paths affected
* GetPath(int index) // returns the affected path
* PatchPath(int index, TCHAR * target) // applies the patch for the
path and saves the result in 'target'
The names of the class methods are actually a little bit different,
but that's pretty much what we have right now and what we need.

What about if you just parse the patch using the diff parser
we have in libsvn_diff? Would that be enough?
See this file for details:
http://svn.apache.org/repos/asf/subversion/trunk/subversion/include/private/svn_diff_private.h
We could make those functions public if you want.

Yes, those functions and structs would suffice to get all the target paths of the patch file.

Another thing: Why do you need to apply the patch to some tempfile?
Why is the preview of the patched result really necessary?
Why not just show the user the patch? When reviewing what a patch
does, it's much easier to look at the patch itself rather than the
patched result -- at least from my point of view, I don't know if your
users are different.

If you agree to these points, we could drop the svn_client_patch() API
change again.

Reviewing a patch file is not what most users expect and are familiar with. Especially if there are many changes spread through a big file, it's not enough to just see the patch file with the three context lines to really get what the changes do - you have to see the full file with all the changes.

Applying the patch to a temp file allows me to show the changes in TortoiseMerge, the old and new file side-by-side. That's what users are used to and where they can really see the result.

Also, if you just review the patch file itself, there's no guarantee that when the patch is applied that that result is what you expect: depending on the algorithm to find the position of where to apply a hunk, such a hunk can get applied somewhere you didn't expect. So a 'real' preview is necessary.

Stefan


--
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net

Reply via email to