Lars Gullik Bjønnes wrote:
Abdelrazak Younes <[EMAIL PROTECTED]> writes:
| | #include "frontends/Alert.h"
| +#include "frontends/Clipboard.h"
|  #include "frontends/Dialogs.h"
|  #include "frontends/FileDialog.h"
|  #include "frontends/font_metrics.h"
|  #include "frontends/LyXView.h"
| -#include "frontends/LyXScreenFactory.h"
| -#include "frontends/screen.h"
| +#include "frontends/Gui.h"
|  #include "frontends/WorkArea.h"
| -#include "frontends/WorkAreaFactory.h"
| +#include "frontends/Painter.h"

Can you sort the includes please?

Sure.

|       // Update from work area
| -     work_area_width = workarea().workWidth();
| -     work_area_height = workarea().workHeight();
| +     work_area_width = workarea().width();
| +     work_area_height = workarea().height();

Have temporary for workarea somehwere?

OK.

| | // Build temporary cursor.
| -     cmd.y = min(max(cmd.y,-1), workarea().workHeight());
| +     cmd.y = min(max(cmd.y,-1), workarea().height());

spacing

You mean:

        cmd.y = min(max(cmd.y, -1), workarea().height());

??

| Index: src/BufferView_pimpl.h
| ===================================================================
| --- src/BufferView_pimpl.h    (revision 14120)
| +++ src/BufferView_pimpl.h    (working copy)
| @@ -35,25 +35,33 @@
|  #include <boost/signals/trackable.hpp>
| | | +

Do we need three empty lines?

As in "I don't like too much spacing" ?
No, I suppose... I'll remove that.

| @@ -179,8 +192,10 @@
|       std::vector<Position> saved_positions;
|       ///
|       void menuInsertLyXFile(std::string const & filen);
| -     /// our workarea
| -     WorkArea & workarea() const;
| +
| +     lyx::frontend::WorkArea * work_area_;

Do we want bald pointers? Even just as cache?

I can't use reference because I want to be able to change work_area_ at run-time. This work_area_ is not created nor deleted here so it is a _safe_ use of pointer. Feel free to suggest something else that retain polymorphisms.


| Index: src/frontends/gtk/GView.h
| ===================================================================
| --- src/frontends/gtk/GView.h (revision 14120)
| +++ src/frontends/gtk/GView.h (working copy)
| @@ -12,9 +12,12 @@
|  #ifndef GVIEW_H
|  #define GVIEW_H
| | -#include "frontends/LyXView.h"
|  #include "bufferview_funcs.h"
| | +#include "frontends/LyXView.h"
| +
| +#include "TheGui.h"

This "TheGui" thing sounds/looks strange. what is it? Can it get a
more descriptive name?

Gui.h is the virtual interface to everything Gui related:
- clipboard
- work areas
- views (including menubar, etc)
- etc

There should be only _one_ instance of Gui object, hence the name:
  _The_ Gui


| | // ENCODING: we assume that the backend passes us ISO-8859-1 and
|  // convert from that to UTF-8 before passing to GTK
| -void GWorkArea::putClipboard(string const & str) const
| +void GWorkArea::putClipboard(string const & str)
|  {

Why the const removals?

I can put it back but the const was and still is _wrong_. PutClipbard modify the clipboard and for now, the gtk clipboard is part of GWorkArea so it should not be const. It used to work without the const because the clipboard is not really part of it and putClipboard() just modify some global variable.

In any case, this GWorkArea needs to be cleanup in order to take advantage of the new architecture: It should be split GuiWorkArea and GuiClipboard.


|   *
| - * \author Lars Gullik Bjønnes
| + * \author Lars Gullik Bjønnes
|   * \author John Levon
|   * \author Abdelrazak Younes
|   *

Hmm... your compiler mangles charsets.

Not my compiler but my former editor. Georg has corrected that already.

| +string const internalLineEnding(string const & str)
| +{
| +#ifdef Q_WS_MACX
| +     // The MAC clipboard uses \r for lineendings, and we use \n
| +     return subst(str, '\r', '\n');
| +#endif
| +
| +     return str;
| +/*
| +#ifdef Q_WS_WIN
| +     // Windows clipboard uses \r\n for lineendings, and we use \n
| +     return subst(str, '\r\n', '\n');
| +#endif
| +*/
| +}
| +
| +
|  #ifndef I_AM_NOT_AFRAID_OF_HEADER_LIBRARIES
|  #if USE_BOOST_FORMAT

Are thise releated to the gui changes? Why?

It is part of the cleanup. Those #ifdef where in QWorkArea clipboard code (qt3 and qt4). Instead of putting them back in the new GuiClipboard class I decided to make two helper functions instead.


(and I donÃ't like the IFDEF implementation. Should be broken out into
an win file and a mac file.

Are you suggesting tree additional file that will be compiled depending on the platform? Something like:
- lstring_win.C
- lstring_mac.C
- lstring_x11.C

I am fine with that but I don't know enough about m4 to do this. This can be done after the merging.

So I correct those thing that you don't like can I merge to trunk?

Abdel.

Reply via email to