*** Bug 492654 has been marked as a duplicate of this bug. ***

*** Bug 492653 has been marked as a duplicate of this bug. ***

Hi James!

Could you give us a short status of your work? It would also be interesting what method/signals you will need for the file-manager stuff so that we can add them.

Thanks,
Johannes

Hi Johannes,
It's actually pretty much done; it's totally feature-complete. The only things left to do now are code and namespace cleanups. I know that I've been pretty crappy about status updates, but school gets in the way a lot of times and I'm really only working on it here and there. ;-) 

As for the FM, I'm just going to need some way to set information based on a full path name, as that's what subversion gives us. For example, if I had a path like /home/jim/Projects/anjuta/Makefile.am, and it had a status of modified, could we have a function that takes this information and sets the entry in the FM for that file given it's full path, with a letter or icon next to its entry in the FM that shows the status?

Also, does the FM have some kind of signal for letting plugins know when it's been refreshed?

On that note, have you and Naba worked out your disagreements about the FM yet? I'd rather not have to do this twice. :)

I'll take a look at the FM and let you know of any more things that I might need.

And, I'll probably have an alpha ready (no FM stuff) by today or tomorrow.

Hi!

> As for the FM, I'm just going to need some way to set information based on a
> full path name, as that's what subversion gives us. For example, if I had a
> path like /home/jim/Projects/anjuta/Makefile.am, and it had a status of
> modified, could we have a function that takes this information and sets the
> entry in the FM for that file given it's full path, with a letter or icon next
> to its entry in the FM that shows the status? 

Not yet. An interface could of course be ianjuta_file_manager_add_status (IAnjutaFileManager ifile, GdkPixbuf* status, GError** e) if this would be ok for you. The problem of the implementation is that the file might not be shown in the moment and has to be cached. In this situation I would prefer that the file-manager would emit a "refresh-file" signal (something like void refresh_file (IAnjutaFileManager* ifile, const gchar* uri) and the version control plugin would set the status. But this should be pretty fast because otherwise it would delay the file-manager a lot. In the case the version control system is slow we have to find some asynchronous way. 

> On that note, have you and Naba worked out your disagreements about the FM yet?
> I'd rather not have to do this twice. :)

Yes, you can assume that the new file-manager (from trunk) is used in the future.

If you have time - could you attach a patch even if the namespacing and other little things are not perfect yet. This would make reviewing a lot easier.

Regards,
Johannes

(In reply to comment #5)
 
> Not yet. An interface could of course be ianjuta_file_manager_add_status
> (IAnjutaFileManager ifile, GdkPixbuf* status, GError** e) if this would be ok
> for you. The problem of the implementation is that the file might not be shown
> in the moment and has to be cached. In this situation I would prefer that the
> file-manager would emit a "refresh-file" signal (something like void
> refresh_file (IAnjutaFileManager* ifile, const gchar* uri) and the version
> control plugin would set the status. But this should be pretty fast because
> otherwise it would delay the file-manager a lot. In the case the version
> control system is slow we have to find some asynchronous way. 
I'll go for an interface, and a "refresh-file" signal seems reasonable enough for what I need. As for the asynchronous thing, I've put together some things for the Subversion plugin that might help us out. 

> Yes, you can assume that the new file-manager (from trunk) is used in the
> future.
Great, thanks a lot. 

> If you have time - could you attach a patch even if the namespacing and other
> little things are not perfect yet. This would make reviewing a lot easier.
Sure, just be warned that it's a lot more than just a patch though. ;-)

Created attachment 98435 [details]
Alpha subversion plugin

As per Johannes's request, here's an alpha version of the subversion plugin that I've been working on. It implements all features described above. But there are some things that need some work:

- Johannes's name needs to be added to some copyright sections for some files that are based on his original implementation. (I'm putting that here so I don't forget about it and leave him out.)

- Namespace cleanups. Some time ago, Naba and I discussed putting the various command classes in libanjuta. As such, they'll need to be moved there and be changed over to the Anjuta namespace. Also, the new widget that I created for this, VcsStatusTreeView, needs to be moved over too. 

- Error reporting. Right now it doesn't report errors to the user. On that subject, the error mechanism needs to support the linked list scheme that Subversion uses for errors. The commands currently record errors, but only the first one, and in some cases there is more than one error generated by a failed command.

- Miscellaneous code cleanups. There's a lot of cleaning to be done in the SvnCommand class (mostly the fallout from putting the auth functions in there.) Also, it might be a good idea to take all the notification functions in each class and combine them into one function someplace. However, there's one problem that needs to be addressed first: Since Subversion uses an odd numbering scheme to deprecate things, and then they go about changing function prototypes for callbacks, we probably want to try to use the newer versions of all commands so that we can have one function for all callback events, instead of one function for any command that listens for events like I have now.

To apply:
1. Run make distclean
2. Apply subversion.diff to a fresh checkout of svn trunk.
3. Replace the existing subversion plugin folder with the one in the tarball.
4. Put all the files in /libanjuta into /libanjuta in your tree.
5. Run autogen.sh again and rebuild. 

I hope you enjoy using the new plugin. I'm looking forward to your comments and suggestions.

Thanks,
James

Created attachment 98437 [details]
Alpha subversion plugin (this time with all the files :)

Ignore the last tarball...I forgot the glade catalog file :( The instructions still apply though...

OK, some comments on the patch:

- To make our life easier it would be nice if you could just send a diff. Subversion allows you do run svn add/remove commands on the file that you added/removed in your offline copy even if have no commit access. Use that you create a single .diff file. That works as long as you did not modifiy binary files but I guess you haven't done that.

- Why do you put the svn tree widget in libanjuta? It does either belong to some generic version control plugin or in the svn plugin. But I hope that we can get rid of it and merge it with the file-manager. Do we need multiple selection then?

- For the methods, you may use the new ones where appropriate at least if you can make sure that all major distributions (Ubuntu, Fedore, SuSE) ship versions of the library that support the new ones.

Don't bother too much about a copyright...

(In reply to comment #9)
> - To make our life easier it would be nice if you could just send a diff.
> Subversion allows you do run svn add/remove commands on the file that you
> added/removed in your offline copy even if have no commit access. Use that you
> create a single .diff file. That works as long as you did not modifiy binary
> files but I guess you haven't done that.
Ok, I'll try that.

> - Why do you put the svn tree widget in libanjuta? It does either belong to
> some generic version control plugin or in the svn plugin. But I hope that we
> can get rid of it and merge it with the file-manager. Do we need multiple
> selection then?
Two things:
1. The selection widget isn't svn specific. I designed it to be generic so that it could be used with any VCS. I intend to use it with the CVS plugin as well, and I hope that it could be useful for any other plugin we might implement in the future, like git. Some other widgets are in there, so I thought this would be a nice place for it. However, if you prefer, I could make a separate library for it and put it someplace. 

2. As for the per-file selection thing, on discussion with Naba and Seb we agreed that this would be better than putting it in the FM. Seb preferred that selection be done in a subversion dialog rather than the FM. Originally he suggested that I embed an FM-like widget in each dialog, where a user could find the files needed and select them. But, I thought that to be a little too complicated, so we compromised on something like what TortoiseSVN does: http://tortoisesvn.tigris.org/commitdialog.html. I talked about it with Naba a few days after that and I got the impression that he tended to agree with Seb's view.

> - For the methods, you may use the new ones where appropriate at least if you
> can make sure that all major distributions (Ubuntu, Fedore, SuSE) ship versions
> of the library that support the new ones.
Ok. I'll look into that. 

Thanks for your comments.

James

Created attachment 98499 [details] [review]
Alpha subversion plugin (one big patch)

Johannes: Here's one big patch as you requested. I'm obsoleting the tarball because remove doesn't work in it. This one should work fine.

First thanks for the diff, I will have a look at it soon!

Regarding the tree widget I guess that I missed the point. Should be OK to include this in libanjuta (if we get more widgets later we could move them to libanjuta/widgets anyway).

Thanks!

Wow, this is absolutely awesome! Really, really nice work!

Some little things I noticed:
- We need some kind of progress indication in the commit/log dialog because it is bad when the user has to wait for something to appear and has no indication that something is happening. You can use a simple progress bar here (gtk_progress_bar_set_pulse) or maybe some kind of GtkTreeRendererProgress were appropriate

- I think we need some better options for the diff dialog, maybe combined with the log dialog so that you can select different revision. But that has no priority for now - just to not forget about it.

Would be nice if you could clean up the little things you mentioned because otherwise, the coding style looks solid.

Thanks a lot and keep up that good work!

(In reply to comment #13)
> - We need some kind of progress indication in the commit/log dialog because it
> is bad when the user has to wait for something to appear and has no indication
> that something is happening. You can use a simple progress bar here
> (gtk_progress_bar_set_pulse) or maybe some kind of GtkTreeRendererProgress were
> appropriate
Yeah, this is on my list as well, don't know why I didn't put it on my list, but whatever...anyway, we're probably going to have to use pulse, as SVN gives us absolutely no frame of reference for progress, unfortunately. However, I think a dialog is a little too obtrusive. Would you allow me to patch AnjutaStatus to support pulse progress? I think the status bar is a good place because it's out of the way, doesn't block the user, yet you can still see that a long task is going on in the background. 

A dialog for the progress bar would really be a bad idea. Feel free to patch AnjutaStatus because a pulsing bar might be useful in more cases. But be careful that AnjutaStatus is also used for the progress bar on start up...

I am not yet sure if this is enough in all cases because the user might be focused on the dialog instead of the main window in which case we might want to put the progress bar on the dialog but let's see.

Created attachment 98759 [details] [review]
Alpha subversion plugin (version 2)

Ok, here's the next version of the patch. This fixes all of the problems that were previously discussed. Also included in this patch is support for pulsing in the status progress bar, which is used by many of the commands to indicate progress to the user. Right now, diff, cat, log, and commit commands use this, and it will be easy to add this to other commands if needed. 

Johannes: After playing with it a little, just having the pulse in the status bar with a message seemed like enough to me. Have a look at it and tell me what you think.

Enjoy :)

Good work! I committed this to trunk now because I think the code is ready to live in the repository and it will make development and testing easier.

Some points:
- The progress bar looks good but I would also like to have a progress bar indication when the commit dialog is filling the list of modified files because this can take some seconds. (probably also valid for other dialogs)

- In the commit dialog, I would prefer when all files were checked by default because that's the usual case. The killer idea (read: no so important but you would earn you a beer!) for the commit dialog would be a checkbox "Use ChangeLog entry" for the log message that would automaticly copy the last ChangeLog entry into the log field.

- I noticed a crasher in the log dialog. I have no backtrace yet nor a way to reproduce but I will give you detailed information about it once I have them.

IMHO, you should rather work on git than on CVS once the subversion plugin is finished because only very few projects still use CVS (and we have a working - though not nice plugin). But this has to be discussed with Naba and others (same applies for the bounty stuff).

(In reply to comment #17)
> Good work! I committed this to trunk now because I think the code is ready to
> live in the repository and it will make development and testing easier.
Thanks! Having it in trunk really does make things easier. 

> - The progress bar looks good but I would also like to have a progress bar
> indication when the commit dialog is filling the list of modified files because
> this can take some seconds. (probably also valid for other dialogs)
Probably a dumb question, but what do you mean by this? Are you talking about progress bars next to each file entry in the status view? I could do this, however I'm not really sure how useful this would be, because you have to remember that SVN doesn't give us much to go on for progress. In fact, AFAICT it only tells us what files have been committed, and we already show that in the info pane. How would having a bunch of pulsing progress bars in the status view in the commit dialog help? We already let the user know that a potentially long operation is happening. But I'm not a maintainer, so perhaps I'm missing something. 

> - In the commit dialog, I would prefer when all files were checked by default
This would be a one-liner. Coming up.

> The killer idea (read: no so important but you
> would earn you a beer!) for the commit dialog would be a checkbox "Use
> ChangeLog entry" for the log message that would automaticly copy the last
> ChangeLog entry into the log field.
This would actually be a pretty easy fix. All I'd have to do is maintain a temp file in the session directory someplace, and then just read it into the log message text view and then write that message back to the file for safe keeping on commit.
 
> - I noticed a crasher in the log dialog. I have no backtrace yet nor a way to
> reproduce but I will give you detailed information about it once I have them.
Ok, when you can reproduce it, would probably be good if you could file a separate report and make sure I get assigned to it. 

> IMHO, you should rather work on git than on CVS once the subversion plugin is
> finished because only very few projects still use CVS (and we have a working -
> though not nice plugin). But this has to be discussed with Naba and others
> (same applies for the bounty stuff).
Hmmm...interesting point. I'll start a thread on anjuta-devel and see what comes up. 

Thanks,
James

Created attachment 98860 [details] [review]
All files selected in commit dialog by default

Well, not quite a one-liner, but close enough. ;-)

First, congratulation for this plugin, it is really nice.

Then a few comments from my first use:
- It will be better to automatically save the configuration of all dialog boxes. By example, in the dif dialog, the "Whole project" check box is always unchecked and I need to check it each time.
- After making a diff, it would be better to reuse a previous svn.dif editor if it already exists. Currently, it creates a new buffer named svn.dif each time. One even better feature, would be to avoid the dialog "do you want to discard" when you close it but I think it depends on the document manager.
- Checking that all files are saved before doing an operation would be better. I don't know if it's easy to do though.

As you see, these are only very small details. Thanks again for this work.

Hi!

> Probably a dumb question, but what do you mean by this? Are you talking about
> progress bars next to each file entry in the status view? I could do this,
> however I'm not really sure how useful this would be, because you have to
> remember that SVN doesn't give us much to go on for progress.

I think you misunderstood me. When you open the commit dialog after having modified a bunch of files it takes some seconds until the list of modified files is loaded. I would like to have an indication for that in the progress bar.

> Ok, when you can reproduce it, would probably be good if you could file a
> separate report and make sure I get assigned to it. 

I still have no trace but I think a have an idea about it. The problem is that you could close a dialog before it has completely filled it's tree or received some other data happening in a thread. In this case it seems that some callbacks go wrong.

(In reply to comment #20)
> First, congratulation for this plugin, it is really nice.
> 
> Then a few comments from my first use:
> - It will be better to automatically save the configuration of all dialog
> boxes. By example, in the dif dialog, the "Whole project" check box is always
> unchecked and I need to check it each time.

And have it enabled by default (first-time).

> - After making a diff, it would be better to reuse a previous svn.dif editor if
> it already exists. Currently, it creates a new buffer named svn.dif each time.

Actually, I would prefer them separate, because it helps in comparing different diffs if need be. Closing old tabs is easier if we don't want them anymore, but it would be hard to get two different diffs if they all use the same tab. What would help is to name the tabs more sensibly. Perhaps 'svn-r1-r2.diff'?

> One even better feature, would be to avoid the dialog "do you want to discard"
> when you close it but I think it depends on the document manager.

It should be possible to have an api (get/set) in IAnjutaDocument to say 'this document need not save'. But I am worried if it would break consistency with other documents behaviour. I guess not.

> - Checking that all files are saved before doing an operation would be better.
> I don't know if it's easy to do though.
> 

There have been cases where I wanted to see diff without saving the files, but they are not so often. Perhaps having 'Save all documents before diff' checkbox (enabled by default) in diff dialog would do?

> As you see, these are only very small details. Thanks again for this work.
> 

Yes, indeed a great job. Thanks james.

Created attachment 99015 [details] [review]
Adds pulsing progress bars for anything that uses status

Johannes: Ok, now that I get what you're saying, I'll eat my last comments and give you a patch. ;-)

This adds pulsing progress bars just under the status views in the commit, resolve, and revert dialogs that automatically disappear when status info is retrieved.

Thanks - really nice!

(In reply to comment #22)
> > - It will be better to automatically save the configuration of all dialog
> > boxes. By example, in the dif dialog, the "Whole project" check box is always
> > unchecked and I need to check it each time.
> 
> And have it enabled by default (first-time).
For the diff dialog that should be pretty easy. Ditto for update. But, should we have it for the log viewer too? A lot of times I only get logs for directories and files, but I'd say between that and whole project is about 50/50. 

As for other check boxes, AnjutaSession could probably be of use here.

> it would be hard to get two different diffs if they all use the same tab. What
> would help is to name the tabs more sensibly. Perhaps 'svn-r1-r2.diff'?
This has been on my to-do list for a while, I just haven't gotten around to it...:(
 
> > - Checking that all files are saved before doing an operation would be better.
> > I don't know if it's easy to do though.
> > 
> 
> There have been cases where I wanted to see diff without saving the files, but
> they are not so often. Perhaps having 'Save all documents before diff' checkbox
> (enabled by default) in diff dialog would do?
Does the docman have an API for this already? 

> > As you see, these are only very small details. Thanks again for this work.
> > 
> 
> Yes, indeed a great job. Thanks james.
Thanks for your comments. I'm glad you like it. :-)

(In reply to comment #25)

> As for other check boxes, AnjutaSession could probably be of use here.

Yes, using AnjutaSession should be rather easy for that.

> > There have been cases where I wanted to see diff without saving the files, but
> > they are not so often. Perhaps having 'Save all documents before diff' checkbox
> > (enabled by default) in diff dialog would do?
> Does the docman have an API for this already? 

Yes, you can use ianjuta_file_savable_save() on the document manager to save all open documents.

To disabled the question if an individual editor window should be saved you could use ianjuta_file_savable_set_dirty (false) on the editor.

Created attachment 99248 [details] [review]
Whole Project checkbox enabled by default in diff and update dialogs

Created attachment 99250 [details] [review]
More descriptive editor names

This patch changes the editor names for diff editors to something much more descriptive. For example, working copy/head diffs would have an editor with a name like "[Working Copy/Head] some-file.diff," where some-file is the name of the file/directory that was diffed. For diffs against arbitrary diffs the name would look like "[Revisions 2832/3200] some-file.diff" for a diff of some-file between revisions 2832 and 3200. 

As an aside, I also changed the name of the editors for the cat command to be more consistent with the above described style, and have syntax highlighting for these cases.

Naba has decided that I've done enough work for the prize, so I'm confirming that I've received the money and closing this bug. But, could you please make sure to commit my outstanding patches on this bug? As for any other problems/feature requests for the SVN plugin, please file separate bugs.

Thanks,
James

(In reply to comment #29)
> But, could you please make sure to commit my outstanding patches on this bug?
> 
Committed.