You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Martin Hauner <ma...@gmx.net> on 2005/06/08 20:37:29 UTC
proposal: diff preview for gui client
Hello,
on Julians request I retry to gather the requirements for the gui
diff preview. I don't consider diff summarize here so we don't mix
the requirements between the two issues.
I hope this helps to make some progress on this issue. :)
This is about getting a usable gui interface for diffs between urls
like tags or branches.
A usable interface for a gui client is to present a list of modified
items between the diff targets as a diff preview. The user can then
decide on which item she likes to see a detailed diff in a visual diff
tool by running a normal diff on the items of the list.
The diff preview should take the same (diff) parameters as diff and
diff_peg and should provide the following information:
for each modified item:
- if it was added, modified or deleted
- if it has property changes
- the items path relative to its diff target (as in a normal diff)
Possibly two paths: its old and new name if the item was moved or
copied.
- if it is a folder or a file (it doesn't make sense to run a visual
diff on a folder)
- its mime type (the gui could decide based on the mime type which
diff tool to run, ie. binary or text)
There is currently no way to get a diff preview from the svn api. The
only known solution is to parse the text output from diff for the
Index: or ---/+++ lines.
I think the svn api is missing a clean way to provide this information.
In the thread "show diffs in a gui client, how to get a list of changed
files?" people like to see this info or a similar info in the shell too.
To the cli users: Is it the same info like above or is it more like
issue #2015, diff summarize?
Implementation for gui:
To get the information for the gui use case i would like to see two
new api calls svn_client_diff_preview/svn_client_diff_preview_peg or
new parameters to svn_client_diff2/svn_client_diff_peg2 to return the
preview info. I think it would be nice to provide this info through
an svn_client_status like callback and structure.
typedef void (*svn_diff_preview_func_t) (void *preview_baton,
svn_diff_preview_t *diff);
typedef struct svn_diff_preview_t
{
/** file or dir */
svn_node_kind_t node_kind;
/** item paths, relative to the old diff target and new diff target */
const char* path_old;
const char* path_new;
/** the mime types of old and new */
const char *mimetype_old,
const char *mimetype_new,
/** change kind: unchanged, added, modified or deleted */
svn_preview_kind_t preview_kind;
/** properties changed? */
svn_boolean_t props_changed;
} svn_diff_preview_t;
This information is easily provided through a new implementation
of svn_wc_diff_callbacks2_t. The callbacks already provide all the
needed information and could simply create an svn_diff_preview_t
and pass it to svn_diff_preview_func_t.
--
Martin
Subcommander, http://subcommander.tigris.org
a crossplatform Win32/Unix/MacOSX svn gui client & diff/merge tool.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Martin Hauner <Ma...@gmx.net>.
Julia Foad wrote:
> Martin Hauner wrote:
> > This is about getting a usable gui interface for diffs between urls
> > like tags or branches.
>
> > typedef struct svn_diff_preview_t
> > {
> > /** file or dir */
> > svn_node_kind_t node_kind;
> >
> > /** item paths, relative to the old diff target and new diff target */
> > const char* path_old;
> > const char* path_new;
> >
> > /** the mime types of old and new */
> > const char *mimetype_old,
> > const char *mimetype_new,
>
> For a directory, I suppose these would both be null?
Yes.
> >
> > /** change kind: unchanged, added, modified or deleted */
> > svn_preview_kind_t preview_kind;
>
> For a file, I suppose this would refer to its content? That would
> address Steve King's request for a "text_changed" field.
>
> For a directory, what exactly would this field mean?
Yes, for files it is meant for the content.
For a folder this would be:
1. unchanged if there are only property changes or
2. added if added
3. deleted if deleted
> > /** properties changed? */
> > svn_boolean_t props_changed;
> >
> > } svn_diff_preview_t;
>
>
> Apart from those details, the next stage is to work out how the
> client and server need to interact to provide this information,
> and whether new server protocol elements etc. are needed in order
> to get the result efficiently. If so, consider also backward
> compatibility.
oh..
> Of course, you could just implement this crudely on the client side to
> begin with, and leave the efficient implementation as a later task. I
> don't know if people would think that was worthwhile.
.. i thought this would be client side only. I am able to provide a
client side patch. I would like to get a few pointers where to look at
the code if this involves client/server communication.
I don't speak native subversion. ;-)
--
Martin
Subcommander, http://subcommander.tigris.org
a crossplatform Win32/Unix/MacOSX svn gui client & diff/merge tool.
Weitersagen: GMX DSL-Flatrates mit Tempo-Garantie!
Ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Julian Foad <ju...@btopenworld.com>.
Martin Hauner wrote:
> on Julians request I retry to gather the requirements for the gui
> diff preview. I don't consider diff summarize here so we don't mix
> the requirements between the two issues.
>
> I hope this helps to make some progress on this issue. :)
Thanks, Martin. This is an excellent proposal: exactly the level of detail
that I was looking for.
> This is about getting a usable gui interface for diffs between urls
> like tags or branches.
> typedef struct svn_diff_preview_t
> {
> /** file or dir */
> svn_node_kind_t node_kind;
>
> /** item paths, relative to the old diff target and new diff target */
> const char* path_old;
> const char* path_new;
>
> /** the mime types of old and new */
> const char *mimetype_old,
> const char *mimetype_new,
For a directory, I suppose these would both be null?
>
> /** change kind: unchanged, added, modified or deleted */
> svn_preview_kind_t preview_kind;
For a file, I suppose this would refer to its content? That would address
Steve King's request for a "text_changed" field.
For a directory, what exactly would this field mean?
>
> /** properties changed? */
> svn_boolean_t props_changed;
>
> } svn_diff_preview_t;
Apart from those details, the next stage is to work out how the client and
server need to interact to provide this information, and whether new server
protocol elements etc. are needed in order to get the result efficiently. If
so, consider also backward compatibility.
Of course, you could just implement this crudely on the client side to begin
with, and leave the efficient implementation as a later task. I don't know if
people would think that was worthwhile.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Philip Martin <ph...@codematters.co.uk>.
"Martin Hauner" <Ma...@gmx.net> writes:
> Philip Martin wrote:
>> Martin Hauner <ma...@gmx.net> writes:
>>
>> > - if it is a folder or a file
>>
>> A modified item has two types, before and after, they might not be the
>> same.
>
> Ok, we can add another type so have a before and after type.
Thinking again, I might be wrong here--it all depends on how you
intend to handle an item being replaced.
>> > (it doesn't make sense to run a visual
>> > diff on a folder)
>>
>> Why not? One could generate two text files containing the directory
>> contents, one item per line, sort them and then run a textual diff to
>> show items added and deleted. Property changes make as much sense on
>> directories as they do on files.
>
> Wouldn't i need the info file/dir in your case too?
Yes. I don't think I claimed otherwise?
> My point was that
> you would have to know if it is file or folder to decide how to run a
> diff on that item.
Agreed, but that's not quite the same as your original claim which was
that visual directory diffs don't make sense.
>> > - its mime type (the gui could decide based on the mime type which
>> > diff tool to run, ie. binary or text)
>> >
>> >
>
>> > There is currently no way to get a diff preview from the svn api.
>>
>> Do you mean the libsvn_client API? The libsvn_ra API allows it if you
>> write an appropriate svn_delta_editor_t, although it does have to
>> handle text-deltas.
>
> Yes, libsvn_client. Change that to: there is currently no >easy< way.. :)
>
> I don't know how much effort is necessary to create an svn_delta_editor_t.
> Is it an easy task?
Much the same as svn_wc_diff_callbacks2_t, but driving the RA layer
might be a bit more work.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Martin Hauner <Ma...@gmx.net>.
Philip Martin wrote:
> Martin Hauner <ma...@gmx.net> writes:
>
> > This is about getting a usable gui interface for diffs between urls
> > like tags or branches.
>
> Perhaps I don't quite understand what you are proposing, but I suspect
> that if you create this new interface there will be users that want it
> to accept wc paths as well as URLs.
You are right.
> >
> > - if it is a folder or a file
>
> A modified item has two types, before and after, they might not be the
> same.
Ok, we can add another type so have a before and after type.
> > (it doesn't make sense to run a visual
> > diff on a folder)
>
> Why not? One could generate two text files containing the directory
> contents, one item per line, sort them and then run a textual diff to
> show items added and deleted. Property changes make as much sense on
> directories as they do on files.
Wouldn't i need the info file/dir in your case too? My point was that
you would have to know if it is file or folder to decide how to run a
diff on that item.
> > - its mime type (the gui could decide based on the mime type which
> > diff tool to run, ie. binary or text)
> >
> >
> > There is currently no way to get a diff preview from the svn api.
>
> Do you mean the libsvn_client API? The libsvn_ra API allows it if you
> write an appropriate svn_delta_editor_t, although it does have to
> handle text-deltas.
Yes, libsvn_client. Change that to: there is currently no >easy< way.. :)
I don't know how much effort is necessary to create an svn_delta_editor_t.
Is it an easy task?
> > Implementation for gui:
> >
> > To get the information for the gui use case i would like to see two
> > new api calls svn_client_diff_preview/svn_client_diff_preview_peg or
> > new parameters to svn_client_diff2/svn_client_diff_peg2 to return the
> > preview info. I think it would be nice to provide this info through
> > an svn_client_status like callback and structure.
> >
> > typedef void (*svn_diff_preview_func_t) (void *preview_baton,
> > svn_diff_preview_t *diff);
> >
> >
> > typedef struct svn_diff_preview_t
> > {
> > /** file or dir */
> > svn_node_kind_t node_kind;
> >
> > /** item paths, relative to the old diff target and new diff target
> */
> > const char* path_old;
> > const char* path_new;
> >
> > /** the mime types of old and new */
> > const char *mimetype_old,
> > const char *mimetype_new,
> >
> > /** change kind: unchanged, added, modified or deleted */
> > svn_preview_kind_t preview_kind;
>
> What about move/copy? Does the client deduce it from the paths? I'm
> really not sure how move/copy will work.
I'm assuming that this is already handled by the (diff) code that calls
the svn_wc_diff_callbacks2_t functions. So that i could simply copy the
parameters from the callbacks into the preview structure.
> > /** properties changed? */
> > svn_boolean_t props_changed;
> >
> > } svn_diff_preview_t;
> >
> >
> > This information is easily provided through a new implementation
> > of svn_wc_diff_callbacks2_t. The callbacks already provide all the
> > needed information and could simply create an svn_diff_preview_t
> > and pass it to svn_diff_preview_func_t.
>
> I'm not sure how you see this interface being used. How is the GUI
> going to use your new svn_diff_preview_func_t callback? I guess it's
> going to run svn_client_diff_preview to completion, and then allow the
> user to choose modified items based on stored svn_diff_preview_t info.
Yes, like that. Another possibilty would be to add new items on the fly
into the gui while the preview is running to get earlier feedback.
> But I suppose it could give the user a yes/no choice as each callback
> runs?
Yes, that's another posibility. I prefer to provide a list from which
the user can choose. I can quickly go the file i like to see and won't
have to click through all files to get to the wanted file.
> One of the problems of the "easy" implementation is that there is no
> libsvn_ra interface that will do the work without getting the
> text-deltas. That means that svn_client_diff_preview is going to
> receive (and discard?) all the text-deltas. Then when a modified item
> is chosen the client is going to need to need that text-delta again,
> or it's going to need to retrieve both full-texts.
Ah, i do understand now why Julian was talking about client/server
interaction. :)
> Also I'm not sure whether svn_diff_preview_t really needs to provide
> the mime-types. The client will need to retrieve the properties once
> the modified item has been chosen, so the mime-types will be
> available, why does it need to have them in advance?
Yes i could retrieve them later. I included it because the info is
availabe from the svn_wc_diff_callbacks2_t functions. Another reason
is simply that i would need less code in the gui tool. :)
> In the short term I suppose a new svn_wc_diff_callbacks2_t will
> do, in the long term we might need a new RA function that tells us
> which things have changed without sending the details of the changes,
> i.e. no text-deltas (and maybe no property changes either).
+1, if it counts..
--
Martin
Subcommander, http://subcommander.tigris.org
a crossplatform Win32/Unix/MacOSX svn gui client & diff/merge tool.
Weitersagen: GMX DSL-Flatrates mit Tempo-Garantie!
Ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Martin Hauner <ma...@gmx.net>.
Peter N. Lundblad wrote:
> On Mon, 20 Jun 2005, Martin Hauner wrote:
>> [diff review implementation]
>>Is this basically the correct direction?
>
> Seems so, yes.
Thanks for the additional hints.
There is a strange thing about my current naive implementation
of the delta editor, maybe this is something obvious to anyone
here.
The open_file/apply_textdelta methods are called for a few files
that have no differences. The files don't show up when I run the
equivalent svn diff from the command line.
--
Martin
Subcommander, http://subcommander.tigris.org
a cross platform Win32/Unix/MacOSX subversion gui client & diff/merge tool.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 20 Jun 2005, Martin Hauner wrote:
> Peter N. Lundblad wrote:
> > But, as Philip implicitly points out, that will send you text deltas
> > (which you ignore, I guess). To be more efficient, you need a revised (see
> > HACKING) version of svn_ra_do_diff, with a parameter indicating whether to
> > send text deltas or not. We already have that functionality for
> > svn_ra_do_status, so I think it should be pretty straight-forward to
> > implement.
>
> Oh, I see. So from scanning a bit through the source the big picture is:
>
> - add a new svn_ra_do_diff2 method with an additional parameter
> svn_boolean_t text_deltas
>
> - add the new boolean parameter to do_diff in svn_ra__vtable_t
>
Update the compatibility wrapper in wrapper_template.h to always pass
TRUE for this parameter.
> - and finally pass the new parameter to svn_repos_begin_report in the
> different repository access libs.
>
Update the protocols where necessary (at least for ra_svn it is).
> I guess there is some more stuff to do between the last two steps.
>
> Is this basically the correct direction?
>
>
Seems so, yes.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Martin Hauner <Ma...@gmx.net>.
Peter N. Lundblad wrote:
> On Sat, 18 Jun 2005, Martin Hauner wrote:
>
> > Ben Collins-Sussman wrote:
> > >
> > > IIUC, 'svn merge' is now doing exactly this in svn 1.2.
> > > [..]
> > > So in svn 1.2, cmpilato changed 'svn merge' to request one of our old-
> > > fashioned tree-delta reports, one which we used to call a "skelta".
> > > It's a tree-delta without any text-deltas, just a description of
> which
> > > files need to be fetched. 'svn merge' now quickly sucks the whole
> > > skelta into a tmpfile, then parses the skelta slowly, doing
> > > svn_ra_get_file()'s as needed.
> > >
> > > So it sounds like we don't need a new RA function here at all, do we?
> > > 'svn merge' is already doing this very thing, so it must be already
> > > possible.
> >
> > Yes, it can be done without a new ra function.
> >
> > I created a new api function svn_client_diff_preview with a callback
> > function svn_diff_preview_func_t. It uses a simple svn_delta_editor_t
> > implementation as input to svn_ra_do_diff. So no new ra function.
> ...
>
> But, as Philip implicitly points out, that will send you text deltas
> (which you ignore, I guess). To be more efficient, you need a revised (see
> HACKING) version of svn_ra_do_diff, with a parameter indicating whether to
> send text deltas or not. We already have that functionality for
> svn_ra_do_status, so I think it should be pretty straight-forward to
> implement.
Oh, I see. So from scanning a bit through the source the big picture is:
- add a new svn_ra_do_diff2 method with an additional parameter
svn_boolean_t text_deltas
- add the new boolean parameter to do_diff in svn_ra__vtable_t
- and finally pass the new parameter to svn_repos_begin_report in the
different repository access libs.
I guess there is some more stuff to do between the last two steps.
Is this basically the correct direction?
--
Martin
Subcommander, http://subcommander.tigris.org
a cross platform Win32/Unix/MacOSX subversion gui client & diff/merge tool.
Weitersagen: GMX DSL-Flatrates mit Tempo-Garantie!
Ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sat, 18 Jun 2005, Martin Hauner wrote:
> Ben Collins-Sussman wrote:
> >
> > On Jun 12, 2005, at 2:23 AM, Martin Hauner wrote:
> >
> >> Philip Martin wrote:
> >>
> >>
> >>> [diff preview]
> >>> In the short term I suppose a new svn_wc_diff_callbacks2_t will
> >>> do, in the long term we might need a new RA function that tells us
> >>> which things have changed without sending the details of the changes,
> >>> i.e. no text-deltas (and maybe no property changes either).
> >>>
> >
> > IIUC, 'svn merge' is now doing exactly this in svn 1.2.
> > [..]
> > So in svn 1.2, cmpilato changed 'svn merge' to request one of our old-
> > fashioned tree-delta reports, one which we used to call a "skelta".
> > It's a tree-delta without any text-deltas, just a description of which
> > files need to be fetched. 'svn merge' now quickly sucks the whole
> > skelta into a tmpfile, then parses the skelta slowly, doing
> > svn_ra_get_file()'s as needed.
> >
> > So it sounds like we don't need a new RA function here at all, do we?
> > 'svn merge' is already doing this very thing, so it must be already
> > possible.
>
> Yes, it can be done without a new ra function.
>
> I created a new api function svn_client_diff_preview with a callback
> function svn_diff_preview_func_t. It uses a simple svn_delta_editor_t
> implementation as input to svn_ra_do_diff. So no new ra function.
...
But, as Philip implicitly points out, that will send you text deltas
(which you ignore, I guess). To be more efficient, you need a revised (see
HACKING) version of svn_ra_do_diff, with a parameter indicating whether to
send text deltas or not. We already have that functionality for
svn_ra_do_status, so I think it should be pretty straight-forward to
implement.
Best,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Martin Hauner <ma...@gmx.net>.
Ben Collins-Sussman wrote:
>
> On Jun 12, 2005, at 2:23 AM, Martin Hauner wrote:
>
>> Philip Martin wrote:
>>
>>
>>> [diff preview]
>>> In the short term I suppose a new svn_wc_diff_callbacks2_t will
>>> do, in the long term we might need a new RA function that tells us
>>> which things have changed without sending the details of the changes,
>>> i.e. no text-deltas (and maybe no property changes either).
>>>
>
> IIUC, 'svn merge' is now doing exactly this in svn 1.2.
> [..]
> So in svn 1.2, cmpilato changed 'svn merge' to request one of our old-
> fashioned tree-delta reports, one which we used to call a "skelta".
> It's a tree-delta without any text-deltas, just a description of which
> files need to be fetched. 'svn merge' now quickly sucks the whole
> skelta into a tmpfile, then parses the skelta slowly, doing
> svn_ra_get_file()'s as needed.
>
> So it sounds like we don't need a new RA function here at all, do we?
> 'svn merge' is already doing this very thing, so it must be already
> possible.
Yes, it can be done without a new ra function.
I created a new api function svn_client_diff_preview with a callback
function svn_diff_preview_func_t. It uses a simple svn_delta_editor_t
implementation as input to svn_ra_do_diff. So no new ra function.
The delta editor creates svn_diff_preview_t structures with the
requested information and then calls the callback with the created
info structure when close_file or close_directory gets called.
It is still unfinished, it only does repos/repos preview and it doesn't
handle properties so far. Since i started with copying the diff code
(there is a lot of necessary preparation) there is also some work to
extract the common code between diff and diff_preview.
Apart from that it already works quite nice from subcommander :-)
If anyone is interested i can post an unfinished patch for review.
--
Martin
Subcommander, http://subcommander.tigris.org
a cross platform Win32/Unix/MacOSX subversion gui client & diff/merge tool.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> So in svn 1.2, cmpilato changed 'svn merge' to request one of our old-
> fashioned tree-delta reports, one which we used to call a "skelta".
> It's a tree-delta without any text-deltas, just a description of
> which files need to be fetched. 'svn merge' now quickly sucks the
> whole skelta into a tmpfile, then parses the skelta slowly, doing
> svn_ra_get_file()'s as needed.
Are you referring to issue 2048? That's an ra_dav specific change, it
doesn't really change the RA interface to the client.
> So it sounds like we don't need a new RA function here at all, do we?
> 'svn merge' is already doing this very thing, so it must be already
> possible.
I think we need a new RA interface to expose the skelta.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Ben Collins-Sussman <su...@collab.net>.
On Jun 12, 2005, at 2:23 AM, Martin Hauner wrote:
> Philip Martin wrote:
>
>
>> [diff preview]
>> In the short term I suppose a new svn_wc_diff_callbacks2_t will
>> do, in the long term we might need a new RA function that tells us
>> which things have changed without sending the details of the changes,
>> i.e. no text-deltas (and maybe no property changes either).
>>
IIUC, 'svn merge' is now doing exactly this in svn 1.2.
It used to be the case that both 'svn diff' and 'svn merge' requested
a tree-delta from the server which included text-deltas. But this
was sometimes causing timeouts from apache -- apache would detect
zero client reads while the client was busy decompressing text-deltas
and running diff3, and thus apache would forcibly close the connection.
So in svn 1.2, cmpilato changed 'svn merge' to request one of our old-
fashioned tree-delta reports, one which we used to call a "skelta".
It's a tree-delta without any text-deltas, just a description of
which files need to be fetched. 'svn merge' now quickly sucks the
whole skelta into a tmpfile, then parses the skelta slowly, doing
svn_ra_get_file()'s as needed.
So it sounds like we don't need a new RA function here at all, do
we? 'svn merge' is already doing this very thing, so it must be
already possible.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Martin Hauner <ma...@gmx.net>.
Philip Martin wrote:
> [diff preview]
> In the short term I suppose a new svn_wc_diff_callbacks2_t will
> do, in the long term we might need a new RA function that tells us
> which things have changed without sending the details of the changes,
> i.e. no text-deltas (and maybe no property changes either).
I have read through the documentation and I'm trying to understand
what is going on inside subversion. I stepped through diff in the
debugger but I'm still a bit lost.
Can you give me a few hints where to start with a new ra function
that works as described above?
--
Martin
Subcommander, http://subcommander.tigris.org
a crossplatform Win32/Unix/MacOSX subversion gui client & diff/merge tool.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by Philip Martin <ph...@codematters.co.uk>.
Martin Hauner <ma...@gmx.net> writes:
> This is about getting a usable gui interface for diffs between urls
> like tags or branches.
Perhaps I don't quite understand what you are proposing, but I suspect
that if you create this new interface there will be users that want it
to accept wc paths as well as URLs.
> A usable interface for a gui client is to present a list of modified
> items between the diff targets as a diff preview. The user can then
> decide on which item she likes to see a detailed diff in a visual diff
> tool by running a normal diff on the items of the list.
>
> The diff preview should take the same (diff) parameters as diff and
> diff_peg and should provide the following information:
>
> for each modified item:
>
> - if it was added, modified or deleted
>
> - if it has property changes
>
> - the items path relative to its diff target (as in a normal diff)
> Possibly two paths: its old and new name if the item was moved or
> copied.
Move detection might be tricky.
>
> - if it is a folder or a file
A modified item has two types, before and after, they might not be the
same.
> (it doesn't make sense to run a visual
> diff on a folder)
Why not? One could generate two text files containing the directory
contents, one item per line, sort them and then run a textual diff to
show items added and deleted. Property changes make as much sense on
directories as they do on files.
> - its mime type (the gui could decide based on the mime type which
> diff tool to run, ie. binary or text)
>
>
> There is currently no way to get a diff preview from the svn api.
Do you mean the libsvn_client API? The libsvn_ra API allows it if you
write an appropriate svn_delta_editor_t, although it does have to
handle text-deltas.
> The
> only known solution is to parse the text output from diff for the
> Index: or ---/+++ lines.
>
> I think the svn api is missing a clean way to provide this information.
>
>
> In the thread "show diffs in a gui client, how to get a list of changed
> files?" people like to see this info or a similar info in the shell too.
>
> To the cli users: Is it the same info like above or is it more like
> issue #2015, diff summarize?
>
>
> Implementation for gui:
>
> To get the information for the gui use case i would like to see two
> new api calls svn_client_diff_preview/svn_client_diff_preview_peg or
> new parameters to svn_client_diff2/svn_client_diff_peg2 to return the
> preview info. I think it would be nice to provide this info through
> an svn_client_status like callback and structure.
>
> typedef void (*svn_diff_preview_func_t) (void *preview_baton,
> svn_diff_preview_t *diff);
>
>
> typedef struct svn_diff_preview_t
> {
> /** file or dir */
> svn_node_kind_t node_kind;
>
> /** item paths, relative to the old diff target and new diff target */
> const char* path_old;
> const char* path_new;
>
> /** the mime types of old and new */
> const char *mimetype_old,
> const char *mimetype_new,
>
> /** change kind: unchanged, added, modified or deleted */
> svn_preview_kind_t preview_kind;
What about move/copy? Does the client deduce it from the paths? I'm
really not sure how move/copy will work.
>
> /** properties changed? */
> svn_boolean_t props_changed;
>
> } svn_diff_preview_t;
>
>
> This information is easily provided through a new implementation
> of svn_wc_diff_callbacks2_t. The callbacks already provide all the
> needed information and could simply create an svn_diff_preview_t
> and pass it to svn_diff_preview_func_t.
I'm not sure how you see this interface being used. How is the GUI
going to use your new svn_diff_preview_func_t callback? I guess it's
going to run svn_client_diff_preview to completion, and then allow the
user to choose modified items based on stored svn_diff_preview_t info.
But I suppose it could give the user a yes/no choice as each callback
runs?
One of the problems of the "easy" implementation is that there is no
libsvn_ra interface that will do the work without getting the
text-deltas. That means that svn_client_diff_preview is going to
receive (and discard?) all the text-deltas. Then when a modified item
is chosen the client is going to need to need that text-delta again,
or it's going to need to retrieve both full-texts.
svn_client_diff_preview is also going to receieve all the property
changes, although since you propose to include property information in
svn_diff_preview_t this appears to be necessary. However you don't
appear to be storing the changes (and I'm not sure storing them would
be feasible) so once again, when the modified item is chosen the
client is going to have to retrieve the properties again.
Also I'm not sure whether svn_diff_preview_t really needs to provide
the mime-types. The client will need to retrieve the properties once
the modified item has been chosen, so the mime-types will be
available, why does it need to have them in advance?
In the short term I suppose a new svn_wc_diff_callbacks2_t will
do, in the long term we might need a new RA function that tells us
which things have changed without sending the details of the changes,
i.e. no text-deltas (and maybe no property changes either).
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: proposal: diff preview for gui client
Posted by steveking <st...@gmx.ch>.
Martin Hauner wrote:
> This is about getting a usable gui interface for diffs between urls
> like tags or branches.
>
> A usable interface for a gui client is to present a list of modified
> items between the diff targets as a diff preview. The user can then
> decide on which item she likes to see a detailed diff in a visual diff
> tool by running a normal diff on the items of the list.
And, it has to be much faster than doing the normal diff. It's not
necessary to actually diff the files - the server would simply need to
provide the requested information, *not* do the diff and extract the
information from that (I can do that myself in a GUI client, what's
important here is speed!).
[snip]
> Implementation for gui:
>
> To get the information for the gui use case i would like to see two
> new api calls svn_client_diff_preview/svn_client_diff_preview_peg or
> new parameters to svn_client_diff2/svn_client_diff_peg2 to return the
> preview info. I think it would be nice to provide this info through
> an svn_client_status like callback and structure.
>
> typedef void (*svn_diff_preview_func_t) (void *preview_baton,
> svn_diff_preview_t *diff);
>
>
> typedef struct svn_diff_preview_t
> {
> /** file or dir */
> svn_node_kind_t node_kind;
>
> /** item paths, relative to the old diff target and new diff target */
> const char* path_old;
> const char* path_new;
>
> /** the mime types of old and new */
> const char *mimetype_old,
> const char *mimetype_new,
>
> /** change kind: unchanged, added, modified or deleted */
> svn_preview_kind_t preview_kind;
>
> /** properties changed? */
> svn_boolean_t props_changed;
/** text changed? /*
svn_boolean_t text_changed;
This is needed, since there could be changes to the properties *and* the
text in one revision.
>
> } svn_diff_preview_t;
>
[snip]
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org