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
Received on 2010-03-10 22:50:52 CET