Angus Leeming wrote:
Abdelrazak Younes <[EMAIL PROTECTED]> writes:
Please found attached a patch to my personal branch. Basically, I have
completely reorganized the frontend handling and adapted BufferView and
the qt4 frontend to the new API. It compiles fine with scons (thanks to
glob ) and qt4 only. I still cannot start it (infinite loop on
QTimer). This patch is huge because of a lot of renaming.
If you don't like the API or want something to be expressed differently
please tell me.
BufferView.C:
You don't need those two commented out
"using lyx::frontend::BaseWorkArea,Clipboard" lines.
(Code moved to Pimpl.)
Yep, will remove that.
BufferView.h:
Do you need to forward declare "BaseScreen"? Looks like it
isn't used.
Yep, will remove that also.
BufferView_pimpl.C:
I like the look of your changes here. The points that follow
aren't a criticism of what you've done. They're thoughts on
where all this should go in the end.
I agree with all what you are saying below but I'd like to point out
something first: I am taking an evolutionary approach. Multiple views
will come after. If I derive too much from HEAD all my work will be lost
so my goal for now is to put all this in a working shape (without
additional functionality) so that I can merge my branch into trunk.
Then, we can start another short-lived branch for the multi-view case.
Having said that, let me comment on your comments:
+ work_area_id_ = owner_->frontend().newWorkArea(width, height);
+ work_area_ = & owner_->frontend().workArea(work_area_id_);
Is it up to the core to define work area dimensions? I'd have
thought that, given a LyX window, it was up to the frontend
to decide on how much space to give a new WorkArea. Of course,
once a WorkArea exists, the core might reasonably ask the
frontend to create a new WorkArea by splitting an existing
work_area_id_ in half, horizontally or vertically. (Like emacs
does it.)
Or in tabs. I think my first temptative to multiple work areas will use
tabs (a la firefox).
Perhaps I'm imagining an API:
class BaseFrontend {
public:
typedef std::size_t size_type;
static size_type const npos = static_cast<size_type>(-1);
enum SplitDirection {
SplitVertically,
SplitHorizontally
};
/// Create a new work area.
/// if @c id_of_existing_workarea is the idea of an existing work area,
/// then the new work area is created by splitting the existing one
/// in two, either horizontally or vertically as defined by @c splitdir.
/// @returns the id of the newly created work area.
size_type newWorkArea(
size_type id_of_existing_workarea = npos,
SplitDirection splitdir = SplitVertically);
/// @returns the work area with @c id.
/// @throws InvalidArgumentException if no match is found.
WorkArea & workArea(size_type id);
};
If you provide the BufferView::Pimpl::frontend() accessor, do
you also need BufferView::Pimpl::workarea() and
workarea() yes because this saves the look into the map:
frontend().workArea(id);
I didn't want to change bufferview and bufferview_pimpl too much in this
first iteration. workarea() is used quite a lot there. But I could
change all internal use to "work_area_->" instead.
BufferView::Pimpl::clipboard() accessors?
This one can go indeed.
You don't appear to
provide a shortcut to frontend().cursor()...
I could add it if you reckon it's cleaner.
BufferView::Pimpl::frontend().cursor().show(*bv_) takes a
BufferView const & argument. Wouldn't it be more logical to take
a work_area_id_ argument. You're saying "display the cursor
in this work area", no?
Yes, that would be nice but there are some processing done before the
cursor is displayed; have a look at void FrontendCursor::show(BufferView
& bv). Maybe this processing could be transfered to the core somewhere.
I wonder if workarea().greyOut() is something that should be
controlled from outside the frontend? If a workarea exists
and doesn't have a BufferView associated with it, then shouldn't
it grey itself out automatically?
Good question. I have to think about it.
I've always found this API clunky:
void BufferView::Pimpl::cursorToggle()
{
if (buffer_) {
- screen().toggleCursor(*bv_);
+ owner_->frontend().cursor().toggle(*bv_);
From where I'm sat, the core should tell the frontend which
WorkArea is "active" (if any) and then it should be up to the
frontend to show the world a flashing cursor.
/me too! But this is a work for later. The impact on the other frontends
would be too big and I am not willing to do that on all frontend now...
frontends/BaseClipboard.h:
update the doxygen comment "\file Clipboard.h"
frontends/BaseFrontend.h:
update the doxygen comment "\file Clipboard.h". Add yourself
as an author.
Yes, will do.
Is class BaseFrontend complete? How do you propose to access
the "LyX Windows" (class LyXView)? I imagine that the frontend
could have multiple "LyX Windows", each of which could have
multiple "WorkAreas". Would each "LyX Window" would have
its own FrontendCursor? I think that emacs has 1 FrontendCursor
per WorkArea but only 1 cursor is ever active (flashing) at
any one time. The others remain visible but are in an inactive
state.
In fact, maybe you need to think a little about the relationship
between BaseFrontend and LyXView. It seems to me that a LyXView
will actually host one or more WorkAreas but that BaseFrontend
would provide the core with the list of all WorkAreas. Does the
core need to know which WorkAreas are hosted by which LyXViews?
Perhaps not.
I am moving the code step by step. The first step was to move the
WorkArea creation out from the Bufferview. Then I agree with you that we
have to transfer LyXView functionality to BaseFrontend. The Qt4 frontend
will then manages the WorkAreas and the Views.
frontends/BaseWorkArea.C:
In a multiple WorkArea world, is the SplashScreen owned by a
WorkArea or is it owned by the "LyX Window" that hosts one
or more WorkAreas?
I agree that this need to be transfered elsewhere. As I said, step by step.
I think that's enough for you to think about. The rest looks
like cut and paste of existing code ;-)
There is a lot of Cut&Paste indeed but making everything fit together
was quite a hard job ;-)
Thanks a lot Angus, and I am happy that you like where I am heading. But
I would like others to express their opinions too. Lars, JMarc, Georg,
is there any hope that my changes are merged into trunk? I would be very
sad if I wasted my time...
Abdel.
