On 10.03.2010 17:23, Julian Foad wrote:
> Hi, Stefan and Stefan.
>
> Stefan Sperling wrote:
>> On Wed, Mar 10, 2010 at 01:19:37PM +0000, Julian Foad wrote:
>>> Stefan Sperling wrote:
>>>> On Wed, Mar 10, 2010 at 10:36:13AM +0000, Julian Foad wrote:
>>>>> This patch appears to have the filter reversed: it lets the user specify
>>>>> paths that the filter should REMOVE, which IMO is not so useful.
>>>>
>>>> Why isn't that useful? I think either way (include and exclude) is useful.
>
> (Re. issue #3434
> <http://subversion.tigris.org/issues/show_bug.cgi?id=3434>)...
>>> I understand that TortoiseSVN wants the include/exclude functionality
>>> available at the API. I wonder if "supply a list of patterns to
>>> include" is really the best API. If I were writing a GUI that wanted to
>>> show a list of what the patch is about to patch, I would want an API
>>> that told me, first of all, what are the paths that this patch file is
>>> going to patch. Maybe a callback that says: "I'm about to patch the
>>> target file TARGET_ABSPATH. If you want, you can set TARGET_ABSPATH to a
>>> different target, or set it to NULL if you don't want to apply this
>>> file-patch at all." Using that, I could either get a list of all
>>> targets in the patch (and make it a dry-run by returning TARGET_ABSPATH
>>> = NULL on each callback), or control which targets are applied, by any
>>> pattern- or list-matching scheme I like, or apply to different paths, or
>>> just apply the patch normally by not providing a callback function at
>>> all, all with the same simple API.
>>
>> The requirements as given by the TortoiseSVN developers were "I want
>> to pass a list of paths the patch is going to modify, the rest of
>> the paths should be ignored by the patching process".
>
> Yup.
>
> (Stefan K, is that really what you want? You don't need to know what
> paths are in the patch, for example?)
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.
And it would be great if I could tell where to save the result, i.e. not
having the patch applied to the target file but to a copy of that file
instead so I can show the user a preview of the result without actually
modifying the real file yet.
>>> Is there a real practical need to have the include/exclude functionality
>>> in "svn" as well? I can't think of a use that's important enough to
>>> justify it. Can you?
Yes: there are a lot of svn clients out there which could benefit from this.
>> First and foremost, it allows us to test this feature independently
>> of TortoiseSVN. That alone of course does not justify exposing the
>> feature at the CLI level. However, since TortoiseSVN users obviously
>> see the need for this feature (assuming the TortoiseSVN devs didn't
>> just make it up and nobody is actually using this feature of TortoiseSVN),
>> why should we not provide it in the CLI client as well?
>
> The usual reasons. Because lots of GUI features are inappropriate in a
> CLI, and this might well be one such. Because well designed software
> doesn't provide features just because somebody wanted such features.
As one of the TSVN devs I can assure you that we didn't just make this
requirement up: TSVN currently does exactly this, but of course with its
own implementation of 'patch' which isn't that good and only works if
the patch applies cleanly.
>>> GNU patch doesn't have it (and it doesn't have an
>>> "include-only" filter either).
>>
>> Good point. But following that rationale I'd rather drop the feature
>> entirely, even at the API level.
>
> I wouldn't discourage dropping it if Stefan K would be happy with that.
Just because GNU patch doesn't have a feature doesn't mean the feature
isn't useful.
And there's a reason why GNU patch is rarely used on Windows: it's
unusable for most situations.
>> Well, for me the question boils down to:
>>
>> Do we want the --exclude-patterns and --include-patterns options,
>> and if not, what do we tell the TortoiseSVN (and AnkhSVN) devs about
>> their requirement that svn patch should be able to patch targets
>> selectively?
>
> All I know about what they want is what Stefan K wrote in issue #3434:
> "svn_client_patch() should have a 'filter' parameter, specifiying one
> (or more?) paths that will get patched." Nothing more, nothing less.
>
> We seem to be on a roll of adding new command-line options, which are
> undoubtedly useful but I feel it is contrary to one of Subversion's
> original design principles (small command set), and that's making me
> slightly edgy.
Well, all I can say is that without this feature, I can't use the patch
command from the svn library in TortoiseMerge. I would have to keep the
sub-par patch TMerge already has instead, because it has that feature
that is required for previews. And previews are important. Users are
very uncomfortable to apply changes without first knowing what those
changes are and where they go to (i.e., which files are affected).
Sure, if you do an update you also won't know beforehand what changes
are applied to your working copy (unless you run 'svn st -u').
But there you know the people who have commit access. In case of patch
files you don't really know the people who sent you a patch for
something. So reviewing the patch before you apply it to your working
copy (which most likely has local modifications, so reverting isn't
really an option either).
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Received on 2010-03-10 18:56:55 CET