Re: Diff Project --invoke-diff-cmd part

Many thanks for the inspiring feedback!

Gabriela

--------------------------------------------

Julian Foad wrote:

>> * This patch breaks the override --internal-diff for now, because
>> this part has to be revised anyway when the invoke-diff3-cmd part
>> gets added.

> OK, we'll have to decide what should override what. I have not formed
> any opinion on that yet.

Personally I feel that if you define an option on the command line, it
should always override any choice made in the config file for
convenience and flexibility.

Also, I think the --internal-diff-or-merge should turn off whatever
diff/merge arrangements the config file specifies(let's leave the old
command --internal-diff untouched), especially if we add the following
feature:

Paul Watson mentioned on the user list:

http://mail-archives.apache.org/mod_mbox/subversion-users/201304.mbox/%3CDB6B7C8F18A6884EB20D085DA247FBC51779663A%40EVS1.phs.org%3E

that he would like to use mime-types to define what diff program to
use for certain files. (note M. Pilatos' RFC in the thread)

Should the config file store pair values, mime-type and the
invoke-diff(n)-cmd to be used?

New command line design example:

invoke-diff-cmd="---MIME-TYPE=custom_mime_type diffProg ---f1 ....
---MIME-TYPE=default=diff ...."

or, if you just want the diff program you select to be applied to any
mime-type you define:

invoke-diff-cmd="diff ---f1 ..."

The corresponding config file entries would look like:

INVOKE_DIFF_MIME_TYPE_1=myCustomDiffType
INVOKE_DIFF_MIME_TYPE_1_CMD=myDiffProg ---f1 ...

INVOKE_DIFF_MIME_TYPE_2=default
INVOKE_DIFF_MIME_TYPE_2_CMD=myFavoriteDiff ---f1 ...
...
INVOKE_MERGE_MIME_TYPE_1=myCustomMergeType
INVOKE_MERGE_MIME_TYPE_1_CMD=myMergeProg ---f1 ...

I wasn't sure if this kind of dev talk belongs on the user list, so I
CC'ed Paul Watson for now. Let me know if this should be posted to
the existing thread in users as well.

> I think it would be desirable to be able to create arguments
> that contain the substituted file name or label as well as some other
> text, such as "--file1=foo" or "+foo" (which is a form accepted by
> kdiff3). But although forms such as these are accepted by some of the
> diff programs I tried, it wasn't necessary for any of them. I didn't
> come across any other problems.

Just to clarify, would you like "+---f1" expanding to
+diff_file_name or ---f1+foo to diff_file_name+foo respectively?

-----------------------------------------
Danielsh wrote:

> Yes. Also, please document the return value (NULL-terminated argv
array?).

I thought for quite a while that apr_array_header_t should be used
instead of the argv array in svn_io_create_custom_diff_cmd(..) but
then it occurred to me that this is API level and that would force
folks to use APR to access this.

Question 1: does/should svn_io_create_custom_diff_cmd live in the API,
i.e. svn_io.c? It's where it is currently because of scope issues,
but not because I considered it API material, and I think that API
users probably should not have access to this --- I may be wrong, but
how and why would people want to use this function, if they would?

Question 2: Given the additional issue of the diff_cmd_baton data
which is passed piecemeal at times because (I guess) of scope issues,
is there a case for a private diff.h file that covers the diff
universe?

-----------------------------------------
M. Compilato wrote:

> I like the idea! I note that you don't draw attention to any existing
> issues, but you might want to review issue #2044 and see if your work
> resolves the request there:

> http://subversion.tigris.org/issues/show_bug.cgi?id=2044

Yes, the new command fixes this.

> Also, a minor design nit (sorry, no code review): The "---f1"
> construct is something I've never seen before.

That's why I picked it --- I checked extensively, and no-one uses a
triple dash, so it does exactly what we hope it will: never interfere
with anything that people might do. Also, I think it looks quite
'unixy' and it's easy to read. I expect fewer problems on windows.
Received on 2013-04-23 12:34:26 CEST