You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Gabriela Gibson <ga...@gmail.com> on 2013/10/26 23:59:57 UTC

New invoke-diff-cmd feature: --svn_cfg-file and --svn-cfg-file-query

Hi,

I added an internal mechanism to the --invoke-diff-cmd which allows the 
user to automatically or interactively select individual files and their
respective diff cmds, configurable via an arbitrary config file.

There are now two optional, internal switches to the invoke-diff-cmd 
command:

        --svn_cfg-file and --svn-cfg-file-query.

The usage in each case is:

--invoke-diff-cmd='(--switch) (path/to/config/file) (default-diff-cmd)'

Those two switches do the following:

1) --svn-cfg-file causes svn to check the given diff_cmds_config file
    and apply the rules therein automatically on a per-file basis.

    If a file is not specifically mentioned in the svn-cfg-file, then
    the given --invoke-diff-cmd is applied.

2) --svn-cfg-file-query works like --svn-cfg-file, but it will prompt
    the user for every file in the given config-file whether they want
    to use the instruction in the config file, the default
    invoke-diff-cmd or input something completely different.

    Note: --svn-cfg-file-query is not implemented yet.

The config file itself looks like so:

# Note the test1 label
subversion/svn/svn.c = diff -L "test1" %svn_old% %svn_new%
# Testing with more than one word in the label
subversion/libsvn_subr/io.c = diff -L "test 2" %svn_old% %svn_new%

The code itself has a few efficiency issues, but is sufficient for a
proof-of-concept demonstration.

The revision is located here:

http://svn.apache.org/viewvc?view=revision&revision=r1536020

I sent an old copy of the BRANCH-README, the up-to-date version that
describes the new feature is here:

http://svn.apache.org/viewvc?view=revision&revision=r1536037

Thanks for looking!

Gabriela

Re: New invoke-diff-cmd feature: --svn_cfg-file and --svn-cfg-file-query

Posted by Gabriela Gibson <ga...@gmail.com>.
On 30/10/13 17:21, Julian Foad wrote:
>>
 > This new config-file feature. I think the basic idea is totally
 > needed: to be able to configure that different diff
 > programs (and/or with different arguments) should be invoked for
 > different kinds of files. For example: use oodiff[1] for Open
 > Document Text docs, ImageMagick "compare"[2] for images,
 > and "kdiff3" for plain text files and everything else.

Also, adding the internal svn diff/merge as an option would also be
useful -- for example, when I merged my branch initially, to my horror,
kdiff3 showed me 30 conflicts, but the internal svn merge only had 2
conflicts (for which I was very grateful!)

So being able to select a visual tool only when you really need
it instead of getting prompted 30 times is preferable, likewise,
as a bonus service, if svn could the leg work for the user and
select the best merge tool to use that would be great.

If kdiff3 finds 30 conflicts and svn merge only produces 2, it
would save a lot of time if svn should inform me of that fact
and, make the optimal choice for me, maybe even interactively if
I ask it to.  (Say, lets' call this feature 'opti-merge') Not
saying this kind of thing is part of this feature, but looking
ahead I can see something like that building on the diff-config
file concept.

As an aside (because it's handy to have) here is the merge of my
branch that produced this situation:

svn co -r r1526439  https://svn.apache.org/repos/asf/subversion/trunk/ trunk
svn co -r r1502389 
https://svn.apache.org/repos/asf/subversion/branches/invoke-diff-cmd-feature 
branch

$ kdiff3 --version
Qt: 4.8.4
KDE Development Platform: 4.10.5
kdiff3: 0.9.97 (32 bit)

 >> There are now two optional, internal switches to the invoke-diff-cmd 
command:
 >>
 >>       --svn_cfg-file and --svn-cfg-file-query.


 > Maybe you did it that way because it meant less code churn or an
 > easier starting point for you?

I coded this pretzel because it kept the the patch small and made
changing svn.c unnecessary, and so makes it easier to review
given that it's just a proof on concept demo.

 > At the very least a filename pattern with wildcards is
 > needed (for precedent, see the auto-props configuration).

 > More than that, I think it would be nice to be able to match on
 > the value of svn:mime-type and/or other conditions to make it a
 > bit more powerful than just filenames, but just filename matching
 > would be powerful enough initially.

*nod*

 > You mentioned to me that you have specific ideas about why you
 > want a separate config file rather than embedding the
 > configuration in the '~/.subversion/config' file like we do with
 > autoprops configuration. Basically it's because diff preferences
 > can vary per project, which I agree with. Could you elaborate a
 > bit, here?

* It's safer and easier to just tell svn which diff-config file
   to employ, at the point of use.  Every time someone edits or
   swaps out the ~/.subversion/config file, they run the risk of
   mistyping or miscopying.

   Also, even if it only takes 2 minutes to set up, it this occurs
   1000 times, that's 33 man hours wasted on a boring task.

*  ~/.subversion/* isn't versioned since it's not part of a trunk
   but the personal workspace set-up.

   There may be situations where different setups are required --
   users may work on more than one trunk revision or also on
   different projects that use svn.

* The current design is easily managable and also scriptable,
   moreover it's versionable if that is desired.

* Individual users may have different ideas as to what constitutes a
   decent diff program, so this accommodates everyone.

At this point, it's probably best if we split the branch up here,
and just have the invoke-diff-cmd branch for the basic diff part,
ready to go into trunk once everyone is happy with it, and make
two new branches: invoke-merge-cmd-feature and diff-config-feature.

Gabriela

Re: New invoke-diff-cmd feature: --svn_cfg-file and --svn-cfg-file-query

Posted by Julian Foad <ju...@btopenworld.com>.
Hi Gabriela.

It looks 
to me like the basic part of this configurable-diff work (without the 
cfg-file feature) is
 about done, at least to proof-of-concept level, and should soon be 
ready to merge to trunk once we finally settle the syntax and parsing 
rules. I have finally 
got around to trying and reviewing it, and I'll post about that 
separately. I think it would be great to get to that stage: getting it 
in trunk implies it's all agreed, tested, and will be released and 
supported. I suppose you need feedback from me and others in order to 
progress in that direction. And so, meanwhile...

Gabriela Gibsonwrote on 2013-10-26:

> I added an internal mechanism to the --invoke-diff-cmd which allows the user to 
> automatically or interactively select individual files and their
> respective diff cmds, configurable via an arbitrary config file.

This new config-file 
feature. I think the basic idea is totally needed: to be able to 
configure that different diff programs (and/or with different arguments)
 should be invoked for different kinds of files. For example: use 
oodiff[1] for Open Document Text docs, ImageMagick "compare"[2] for 
images, and "kdiff3" for plain text files and everything else.


> There are now two optional, internal switches to the invoke-diff-cmd command:
> 
>        --svn_cfg-file and --svn-cfg-file-query.
> 
> The usage in each case is:
> 
> --invoke-diff-cmd='(--switch) (path/to/config/file) (default-diff-cmd)'
> 
> Those two switches do the following:
> 
> 1) --svn-cfg-file causes svn to check the given diff_cmds_config file
>    and apply the rules therein automatically on a per-file basis.
> 
>    If a file is not specifically mentioned in the svn-cfg-file, then
>    the given --invoke-diff-cmd is applied.
> 
> 2) --svn-cfg-file-query works like --svn-cfg-file, but it will prompt
>    the user for every file in the given config-file whether they want
>    to use the instruction in the config file, the default
>    invoke-diff-cmd or input something completely different.
> 
>    Note: --svn-cfg-file-query is not implemented yet.

As for the UI, where you specify a --svn-cfg-file switch nested within 
the value of another switch, I can only say seems horribly contorted :-)
 Maybe you did it that way because it meant less code churn or an easier
 starting point for you? It seems to me that one would often want to 
specify either a diff command or a diff configuration file but rarely 
both at the same time, so a separate option would seem logical to me.

> The config file itself looks like so:
> 
> # Note the test1 label
> subversion/svn/svn.c = diff -L "test1" %svn_old% %svn_new%
> # Testing with more than one word in the label
> subversion/libsvn_subr/io.c = diff -L "test 2" %svn_old% %svn_new%

Clearly the configuration file will need to be able to specify which files use which diff command in a way that's a bit more flexible than listing every file individually. At the very least a filename pattern with wildcards is needed (for precedent, see the auto-props configuration). More than that, I think it would be nice to be able to match on the value of svn:mime-type and/or other conditions to make it a bit more powerful than just filenames, but just filename matching would be powerful enough initially.

You mentioned to me that you have specific ideas about why you want a separate config file rather than embedding the configuration in the '~/.subversion/config' file like we do with autoprops configuration. Basically it's because diff preferences can vary per project, which I agree with. Could you elaborate a bit, here?

Thanks for the links, and for your detailed BRANCH-README file that makes it easy to understand everything going on here.

- Julian

[1] <http://craig.mcqueen.id.au/OpenOfficeGraphicalDiff.html>
[2] <http://www.imagemagick.org/Usage/compare/>



> The code itself has a few efficiency issues, but is sufficient for a
> proof-of-concept demonstration.
> 
> The revision is located here:
> 
> http://svn.apache.org/viewvc?view=revision&revision=r1536020
> 
> I sent an old copy of the BRANCH-README, the up-to-date version that
> describes the new feature is here:
> 
> http://svn.apache.org/viewvc?view=revision&revision=r1536037
> 
> Thanks for looking!
> 
> Gabriela
>