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.)
BufferView.h:
Do you need to forward declare "BaseScreen"? Looks like it
isn't used.
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.
+ 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.)
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
BufferView::Pimpl::clipboard() accessors? You don't appear to
provide a shortcut to frontend().cursor()...
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?
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?
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.
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.
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.
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 think that's enough for you to think about. The rest looks
like cut and paste of existing code ;-)
Regards,
Angus
