John Levon <[EMAIL PROTECTED]> writes:
| > | // FIXME: do we need to use return value from classApply() here
| > | // ??
| > | classApply();
|
| which indicates that perhaps we need to consider return value, and is
| future proof. Imagine somebody new coming across "// boo succes = //
| usused" - it means NOTHING to them.
ok
| This is a personal peeve of mine - mainly because I have had this exact
| same problem with tons of core lyx code :(
|
| > | -void QCommandBuffer::complete_selected(const QString & str)
| > | +void QCommandBuffer::complete_selected(QString const & str)
| > |
| > | You've tested this part is safe ?
| >
| > how to test?
|
| type "ma" in minibuffer, press right arrow, select an entry.
will do.
| > | If you're going to get rid of the debugging comments while it's still
| > | being worked on just remove them :)
| >
| > qWarning("%d", s) does not compile cleanly... s is QString...
| > (so how can it give valid debug info?)
|
| then please remove it, don't comment it. We are using CVS, remember.
well...
| > | Please don't remove this comment - it is unusual behaviour.
| >
| > Then perhaps the comment should spell it out a bit better.
|
| Probably.
|
| // note we ignore text_ here (non-strict ==), because
| // text_ is not filled out by keymap initialisation
ok
| > | - // Yes, it is this ugly. Two reasons - root items must have a QListView
|parent,
| > | - // rather than QListViewItem; and the TOC can move in and out an
|arbitrary number
| > | - // of levels
| > | + // Yes, it is this ugly. Two reasons - root items must have
| > | + // a QListView parent,
| > | + // rather than QListViewItem; and the TOC can move in and
| > | + // out an arbitrary number of levels
| > |
| > | Why did you fubar the formatting ?
| >
| > Some stupid ass used +80 char lines...
|
| Yes I am stupid ass. But where is emacs format-paragraph ?? Doesn't it
| work :)
|
| // Yes, it is this ugly. Two reasons - root items must have a
| // QListView parent, rather than QListViewItem; and the TOC can
| // move in and out an arbitrary number of levels
|
| That is the result of "gqap" in vim
ok ok ok I will reformat it a bit nicer.
| > | + // FIX! (Lgb)
| > | +
| >
| > the key translating? it should use a map or something so that binary
| > search could be used.... now it is tens and tens of comparisons for
| > the entries close to the bottom.
|
| OK, sure. This stuff is still under heavy tweaking, I haven't cleaned
| up yet.
ok
| > And I had a lot of fun... but the Qt code was in a lot better shape
| > than I would have thought.... good work.
|
| There's still a lot more cleanup I think we can do ;)
Sure, but not formatting wise :-)
Also from the size of the xforms and qt binaries it seems that some
cleanup could be done:
size build/src/lyx qt/src/lyx
text data bss dec hex filename
2516363 80524 48668 2645555 285e33 build/src/lyx
2713142 135952 22972 2872066 2bd302 qt/src/lyx
I would really expect the qt version to be smaler, since qt should be
able to help us out more directly thatn the xforms version.
(this expectation might of course be totaly wrong)
I'll do the above changes and just commit this.
(I guess it is time too look through the xforms and the controller
code as well... I have been too lazy lately)
--
Lgb
