subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan K√ľng <>
Subject Re: svn commit: r919460 - filtering svn patch targets
Date Thu, 11 Mar 2010 18:07:04 GMT
On 10.03.2010 23:22, Stefan Sperling wrote:

>> 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.
> This is really your second request -- you want to get at the
> tempfiles svn patch generates during patching. Have you seen my
> proposal in ?
> Basically, we could have svn_client_patch() look like this:
> svn_error_t *
> svn_client_patch(const char *abs_patch_path,
>                   const char *local_abspath,
>                   svn_boolean_t dry_run,
>                   int strip_count,
>                   svn_boolean_t reverse,
> 		 apr_hash_t **tempfiles,
>                   svn_client_ctx_t *ctx,
>                   apr_pool_t *result_pool,
>                   apr_pool_t *scratch_pool);
> If you pass non-NULL for tempfiles, and TRUE for dry-run, you'd get back
> in *tempfiles a mapping {path as in patch file =>  tempfile containing patched
> result}, the tempfiles are left open by svn_client_patch() so you need to
> close them, and the working copy is not modified.

Why are the files left open? If there's a good reason for leaving them 
open, then of course I can close them in TSVN first before using them. 
But it would be nice if the files were already closed.

> Would that suffice to cover base 2?

Yes, that would work.

> What is not possible (without adding the --include-pattern option)
> is selecting which files to patch. Is selecting individual patch
> targets really that important?

Yes, that's very important. I often find that when I get a patch, I only 
want to use part of it because I found that when reviewing the changes I 
have to reject some of those changes.


   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\

View raw message