Created attachment 114100 [details] [review]
Adds the IAnjutaEditorMultiple and IAnjutaEditorMaster interfaces and corresponding document-manager changes

Adds two new interfaces;
   * IAnjutaEditorMultiple
   * IAnjutaEditorMaster
Modifies the DocumentManager plugin to check for EditorMultiple instances and handle them appropriately. 

So far, I have not faced any problems using the Sourceview and Scintilla editors with this patch.

Created attachment 114166 [details] [review]
Updated patch

* Added a lot of memory management. Still to check if it's completely done
* Added 2 functions to the libanjuta.idl, get_page and set_page.

The anjuta-gvim plugin now works with the GIO port. 
svn checkout http://anjuta-gvim.googlecode.com/svn/trunk/ anjuta-gvim

There is currently a bug while trying to close all buffers (because of a untitled buffer). I am working on it. It otherwise integrates pretty well with the document manager. You can move through documents, and open glade files concurrently as well.

Created attachment 114377 [details] [review]
Updated #2

More bug fixes.

Hi!

The plugin seems to be missing the anjuta.gvimrc file which makes it rather useless. If you attach it fast here I might be able to use it for a lightning talk on GUADEC..

Thanks!

Created attachment 115556 [details] [review]
Updated #3

Several memory management fixes.

OK, looks quite good so far. Porblems:

- The editor-multiple patch break gtksourceview message area
- The dirty-indication (*) is not updated correctly
- I still get warnings about GVIM_NOT_READY, plugin should block in this case until the editor is ready.
- It breaks when the last editor is closed and on tried to open another file.

Created attachment 117785 [details] [review]
Editor Multiple set of patches. Fixes sourceview problem.

Follow up, plugin blocking has been added. The memory bug is still not fixed though (noticed when reopening fails)

Created attachment 125157 [details] [review]
Major Rewrite

Further follow up. Lots of changes, but much cleaner and smaller patch:

In summary, I have removed the hard reference of "document" in the "page", and instead store them separately in a GPtrArray (doc_table), and a GHashTable (doc_page_table). I felt that this was the best way to separate the "page" and the "document", which led to a *lot* of messy code in my editor_multiple patches. IMHO, the patches also simplify some sections of the code.

Created attachment 125158 [details] [review]
Stripped down patch moving documents into an array

This is the above patch without the editor multiple changes. It only separates the document from the page.

Hmm, I don't really get why there are two lists, the hashtable and the ptr-array. Wouldn't one list be enough.

Another note: The indentation in your patch is broken, you should always indent with tabs and not with spaces. (you will see it if you look at the patch file).

Created attachment 125313 [details] [review]
Updated to use only a HashTable and to use tabs instead of spaces.

You are right, I don't need two lists. I had kept it that way for an earlier implementation (which I now realize is obsoleted by using g_hash_list_get_keys()). Here is an updated patch, with all the spaces converted to tabs as well.

Created attachment 125931 [details] [review]
Updated Patch to SVN rev 4548

I've updated the patch to fix a conflict in action-callbacks.c (it was a fix in a reimplementation that was already fixed...)

There is a problem with the Documents menu action items (Previous Document/ Next Document), but I have not changed any of that code (the same happens even without my patches). I will file / confirm that as a seperate bug.

I think the name of the interface IAnjutaEditorMultiple and IAnjutaEditorMaster should be changed. You have written this for an Editor but the function is similar to the IAnjutaDocumentManager interface.

1 What are the differences between IAnjutaEditorMaster and IAnjutaDocumentManager ?
I think the best solution would be to use the IAnjutaDocumentManager interface and allow nested document manager but I don't know if it's easily possible. I would even prefer to change a bit the IAnjutaDocumentManager interface to merge both if it's not a too big task. Else I think you call this rather IAnjutaDocumentManagerChild or something like this.


2 Why do you need IAnjutaEditorMultiple ?
Isn't it possible to that your editor document object implement AnjutaEditorMaster directly.


3 The hash table used in the document manager to link the document and the page looks fine.


4 On the other hand, I don't really like the way you connect the signal using ianjuta_editor_master_is_registered and ianjuta_editor_master_set_registered. Especially because it appears in the interface. I don't have another proposal though for the moment. Do you have other idea for this ?


5 I have include your patch in my local tree, but I get a crash when I load Anjuta. I think it's linked to the way the tabs are ordered, I have "sorted by alphabetic order". It's a endless loop which make the stack overflow, because reordering the tabs change the tabs order which trigger a new reordering of tabs endlessly. I can avoid it by putting in comment the line 1767 in anjuta-docman.c //anjuta_docman_update_documents_menu(docman);
If you cannot reproduce, I will make more investigation here, it's a nice bug that is easy to get.


I don't know you patch very well yet so I could have easily missed something.

Hey,

1) While I agree that there does seem to be some similarity between the EditorMaster and the DocumentManager (in fact, in the beginning, Johannes and I talked about implementing it as another DocumentManager, but decided against it because it would be unnecessarily complicated and repetitive), it is largely superficial, and the DocumentManager performs a lot more tasks than the EditorMaster. The only purpose of the EditorMaster is to provide an interface for the DocumentManager to deal with multiple-document editors (like vim or emacs), in the same way it deals with other Documents (like the SourceView editor or a Glade sheet). 

I agree, calling it DocumentManagerChild makes more sense, and makes it feel less like an alien body. However, I think that the implementation is still largely the same. 

2) The EditorMultiple is a *Document* that is handled by the EditorMaster, and there can be many of them. In the case of SourceView or Scintilla, the document and the widget are one and the same, whereas here, they are separated.

3) -

4) I don't know how else I can ensure that the Master's signals have been registered so Anjuta can handle them. The EditorMaster is created only once, and that too somewhere away from the DocumentManager. If I could make the DocumentManager create the EditorMaster (I see this as having to create something like an "EditorMasterFactory" ...), and apply callbacks at that point, it would remove this requirement, but I feel that this would make the code less clean.

5) I will check this again. I remember testing this feature in the past, and it worked ok (or actually, it did infinitely loop, but I fixed that). I may have caused a regression there.

Created attachment 126619 [details] [review]
Fixes infinite loop on tab-reorder

This patch fixes the infinite loop that is entered on sorting tabs.

The problem was caused because the GtkToggleAction item is activated the update_documents_menu, which also causes the current document to be set, which finally calls the sort function again. Earlier the setting of the current document was done through gtk_notebook_set_current_page, instead of through anjuta_docman_set_current_document.

I have solved this problem by blocking the callbacks before setting the ToggleAction active.

I've revamped the interface code such that:
EditorMultiple contains just 1 function, get_master

EditorMaster contains 2 functions: get/set current_document.

You can get the patches by doing:
git pull git://github.com/arunchaganty/anjuta.git editor-multiple

I would recommend another branch (say "editor-multiple") though:
git branch editor-multiple
git checkout editor-multiple
git pull git://github.com/arunchaganty/anjuta.git editor-multiple

Closing this one.

The vim integration was a nice idea but in the end it just doesn't fit very well into the anjuta architecture. Thanks again to Arun for all his work but for the time being I doubt that this will ever get merged.