On Fri, 2005-03-11 at 20:45 +0100, Andreas Klostermann wrote:
> I changed a lot of the things that where brought up. No instant apply
> anymore, and the jumpto/back buttons get reset when selecting another
> label from the list.
> I changed the glade file, too. The main reason for not using patches in
> this post is, that this version is a major restructuring of code, and
> probably all changes someone else may have done to my code wouldn't work
> anymore with the patch either. Same for the glade file.
Cool. Some points follow.
When I bring up the dialog, the "Document" combo has nothing selected.
When I select a label, the OK button does not become sensitive. The
Apply button is sensitive all the time.
When I click apply to insert a reference, the reference is inserted
twice.
Is there a reason that the treemodel stuff (column class declarations)
are in the .C rather than .h? It would seem more natural to include
them in the GRef class declaration.
> bufferstore_ = Gtk::ListStore::create(bufferColumns);
> vector<string> const buffers = controller().getBufferList();
> buffercombo_->set_model(bufferstore_);
The way you're doing it right now looks like a memory leak to me (same
in update_labels). Why not just reuse the same model each time:
bufferstore_ = buffercombo_->get_model();
bufferstore_->clear();
(and create it just once in the build function)
You left my name at the top of the files, there's no need for that.
Most the code is ripped from one place or another, it would get
confusing if we kept track of the copy and pasting between frontends :-)
Oh, and it still says "GNote" at the top of the files.
> #include "GRef.h"
> #include "ControlRef.h"
> #include "ghelpers.h"
> #include "insets/insetref.h"
> #include "debug.h"
> #include "buffer.h"
> #include "insets/insetnote.h"
It's better to break up the includes, something like this:
"""
#include "GRef.h"
#include "ghelpers.h"
#include "ControlRef.h"
#include "insets/insetnote.h"
#include "insets/insetref.h"
#include "buffer.h"
#include "debug.h"
"""
Thus you've got GTK specific stuff, then ref dialog specific stuff, then
individual classes that we're using, then lyx-wide stuff.
> }
>
> void GRef::update_labels()
> {
Two newlines in between functions, please.
> if (buffernum<0)
> {
Put the curly brace after the if: "if () {"
> if (buffernum<0)
> {
> buffernum=0;
Spaces in between operators, please:
if (buffernum < 0) {
buffernum = 0;
I know this stuff seems very anal, but it will come quite naturally when
you get used to it, and it is worth having all the LyX code the same in
this respect.
John
