Subject: Re: A Proposal (why we should have setBold(true))
From: sam th (sam@bur-jud-118-039.rh.uchicago.edu)
Date: Wed May 31 2000 - 14:49:36 CDT
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Wed, 31 May 2000 jeff@abisource.com wrote:
>
> there's been a lot of good suggestions in this thread.
> as the author of the code in question, let me try to
> explain what was done and why and perhaps make a few
> suggestions for improvement....
Well, it's certainly good to have your input. You just need to work on
having less to say, so we have less to read. :-)
>
> > Subject: A Proposal (why we should have setBold(true))
> > From: sam th (sam@bur-jud-118-039.rh.uchicago.edu)
> > Date: Sat May 27 2000 - 05:09:23 CDT
> > ...
> > Currently, there is a 1-1 relationship between the text of the attributes
> > of an element in the XML produced by abiword and the representation of
> > formatting in the pt. (actually, the piece table uses a hash table, but
> > the principle is the same.) This makes life very easy for the .abw
> > importer - all it has to do is pass the attributes unchanged into the
> > piece table. However, this means that the means of acessing the piece
> > table are (IMHO) the worst sort of low level routines. It works well for
> > the abw importer, since it doesn't have to get down and dirty
> > parsing attributes, constructing string arrays, and then passing them to
> > the pt.
>
> the PT was designed to handle straight XML with attributes
> and CSS properties -- without any knowledge of
> what the various attr/props meant. the primary
> goal of the PT is to represent the document (both
> as imported and as it is edited) and to be able
> to undo/redo modifications.
This isn't really a problem. However, the current situation (IMHO) has
the following problems:
The PT works *very* close to the AbiWord file format. This has a
number of bad consequences. First, it means that changing the PT
implementation (or the file format implementation) would be very hard.
Second, it means that the PT accessor functions are designed to make life
easy for the AbiWord_1 importer, since it maps nicely to the PT. This
means that if you don't just magically have the formatted strings of
properties, your interaction with the PT is *very* difficult.
>
> > Additionally, the abw file format has very little concept of
> > inheritance. It never has to inherit within the same level, such as a
> > block.
>
> this is a **good thing** after dealing with the HTML
> mess, we didn't want to recreate this. [actually, the
> importer will support this (properly nested <spans>),
> but we flatten them pretty quickly.] also, it was our
> goal that the style mechanism would quickly be available
> (the mechanism is in place, but most of the UI work
> never got done) so that most text in the document would
> just reference a named style (like "Heading1") and not
> be a long sequence of attr/props.
All this is excellent, but it means that there is no good mapping from
something like HTML to the ABW file format (and by extension, the PT).
And since the PT accessor routines are so close to the data, this makes
life difficult if you are trying to convert a different model.
>
> > ...
> > The upshot of this is that I have to keep track of what state everything
> > is in, so that I can regenerate the property list for each new element
> > that I create. Needless to say, this makes for ugly code. However, I
> > don't think this should be the importer's job.
>
> i agree that this is ugly and tedious, but it doesn't
> belong in the PT (proper) -- it's already doing too
> much. and besides, the PT wants to have things nice
> and flat.
>
> > ...
> > However, this additon wouldn't just make my life easier.
> > ...
> > 1 - take a look at _toggleSpan in ap_EditMethods.cpp (line 3415)
> > ...
> > 2 - think about scripting. Eventually, we will want to add a scripting
>
> > From: Eric W. Sink (eric@sourcegear.com)
> > Date: Sat May 27 2000 - 09:26:53 CDT
> > ...
> > Have a look at the toolbar state management code.
>
> > From: Martin Sevior (msevior@mccubbin.ph.unimelb.edu.au)
> > Date: Sat May 27 2000 - 08:13:26 CDT
> > ...
> > As far as I'm concerned you're pushing on an open door. When
> > implementing "Insert Symbol" I would have loved such functions to
> > implement the font querying and changes rather than using props queries.
>
> yes, this is the problem -- we never took the time to consolidate
> the 'command' functions into a single place. the toolbar has
> some, the menubar has some, the status bar has some, the rulers
> have some, the edit methods, and the various importers have some.
>
What *I* think the problem is is this:
The PT looks like (and correct me if I'm wrong) it was designed for the
ABW importer to talk to, and the View. It's interfaces work out nicely
for them. Then everything else accesses the much nicer routines in the
View.
But since the importer, by some other decision, talks directly to the PT.
Therefore, when you have a different importer, life becomes much more
difficult, and you have to construct arrays of strings to pass to the PT.
My argument is that this is just wrong. You should not have to know how
the PT is implemented to talk to it.
I see two solutions to this:
1) Add better accessor functions. (This was what I proposed)
2) Add another abstraction layer for importers (This is what Martin
proposed).
> > From: sam th (sam@bur-jud-118-039.rh.uchicago.edu)
> > Date: Tue May 30 2000 - 02:45:51 CDT
> > ...
> > Currently, the ptbl stuff assumes that the only time you need to muck
> > around with it directly is in the importer(s). All the other fun stuff
> > goes throught FV_View. This is fortunate for everybody else, because the
> > ptbl by itself is pretty useless for individual editing. The reason for
> > this is that FV_View maintains an insertion point (of type
> > PT_DocPosition). Now, if you have the current insertion point, the ptbl
> > suddenly becomes lots friendlier. You can find out stuff about the
> > precise point in the ptbl. However, there's no good way (AFAICT) to
> > discover where you are in the document from the ptbl code. And when you
> > are in an importer, you don't have the luxury of a view.
>
> we also failed to divide the FMT code properly (part of it is
> dealing with editing and selection and part is dealing with
> formatting and drawing)...
>
> > From: Bruce Pearson (BruceP@wn.com.au)
> > Date: Tue May 30 2000 - 12:30:24 CDT
> > ...
> > The DOCUMENT holds the information.
> > The VIEW display a current view of the document. (Toolbars and status bars
> > are also views).
> > The CONTROLLER controls the state of the document. (Toolbars call functions
> > of controller eg. setBold)
>
> yes. we have most of this, but it needs some work.
>
We have this in some places. For toggleBold() (in ap_EditMethods) we have
FV_View acting as both the controller and as the view (of course). In the
importers, we just have the document, with nothing else.
> > Structuring it this way we can create a controller to edit a document and
> > call functions in the controller like setBold, etc to edit a document. This
> > controller can then be used by the importer, scripting language, etc.
>
> yes, this needs to be created and have all of the code in the tool
> bars, menu, edit methods, and fv_view and etc moved/consolidated.
>
> > A document should store a list of views. When a document is modified it then
> > sends a message to the attached views telling the views that they need to
> > update the view of the current document. Right now the view changes the
> > document and then needs to work out how to change its own view of the
> > document.
>
> wait, we already have this. each document has a list of DocListeners
> that receive notification when the document changes, including the
> doc positions and the info about what changed. the doc listener in
> src/text/fmt handles formatting and updating the screen. the goal was
> that we could have multiple different DocListeners which would display
> different types of view (page layout, outline, normal mode, etc).
>
> > The controller stores the current insertion point and also various
> > information like selection start and end points. When this information
> > changes (eg use presses left arrow) the controller does required operations
> > and then tells the view to update its view of the insertion point. i.e. View
> > is also a view of the controller.
>
> we also have this in the text/fmt (primarily fv_view) code,
> but it could be isolated better.
>
> [also, we also have a list of "ViewListeners" which need to respond
> when things like the insertion point or the current selection changes;
> these are hanging off of the view.]
>
> > Implementing this at this stage may or may not be a huge job and may bring
> > up a whole range of other implementation issues. Does any body have any
> > comments. Is this something we need to do now or should other things be done
> > before hand.
>
> i've not been active on the project for a while, so i don't
> know what's pressing or urgent at this time. but, if you
> wanted to create a "controller" class and put a lot of the
> Get/Set methods that various parts of the app needs in it.
> and move the code out of the tool bars, menus, edit methods,
> and etc, that would be a good thing.
>
> what would really help is to look at the naming of things
> in the src/text/fmt directory. yes, we tried to be good here,
> but too much stuff got added and some of it got named poorly.
> (for example, most of what we're describing here as "controller"
> is in FV_View. part of the problem is that "VIEW" is an overloaded
> term.)
>
> now, before we do anything, we need to remember that we
> allow multiple windows on a document and each needs to
> work somewhat independently (multiple insertion points
> and (later) multiple renderings (page-mode vs normal mode)).
> so, we can't just push things down into the PieceTable.
>
> it would be nice if the controller were abstracted
> out of the text/fmt directory and sub-classable. so i
> can have basic cursor motion/selection working for all
> types of "windows" and then be able to sub-class it for
> the quirks of outline-mode...
>
> for now, it's probably best to limit the conversation
> to re-doing text/fmt without worrying about other types
> of views. (one overhaul at a time please :-)
>
> finally, for what it's worth, i know manipulating lists of
> attr/props is a pain-in-the-a$$ and keeping the PieceTable
> ignorant of their meaning is (in some places) painful, but
> it lets us do nice things elsewhere. for example, someone
> was able to add overstrike without worrying about whether
> it broke undo/redo....
>
> so, i think we should cut up FV_View and add some nice
> Set/IsSet verbs (and perhaps rename a few things for
> clarity) and not make any changes to the PieceTable.
>
Let me see if I have your proposal right.
At the bottom layer we have the PT PT
/ \
Then we have two branches View Controller
/ \ \ \
Then we have things that Toolbar Display \ Importers
use these layers \
EditMethods
Currently, the situation looks like this
PT
/ \
Importers View
/ | \
Display | EditMethods
|
Toolbar
So what the change would involve would be a wholesale splitting of
everything in src/text/fmt into two groups, say fmt and ctrl.
All the set/isSet stuff would go in ctrl, and all the rendering stuff
would go in fmt.
This sounds like a decent plan to me. It would address all the
problems I encounterd in the XHTML importer. However, I'm not capable of
doing this myself. I have neither the programming nor the AbiWord
experience to do it. But it should be done, and I am certainly willing to
help as much as possible.
As an additional concern, if we were to do this major conversion, what
would that do to any timeframe people might have. Does sourcegear want to
release abiword in the near future? Would this rewrite hinder that?
Just some questions.
sam th
sam@uchicago.edu
http://sam.rh.uchicago.edu
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.1 (GNU/Linux)
Comment: For info see http://www.gnupg.org
iD8DBQE5NWzSt+kM0Mq9M/wRAvOPAJ9dS5vxeY6Hstf/uOU0Qp6qk61hrwCdEDEu
NrlvLGxz84pq2Ct09FDuTV4=
=r9vT
-----END PGP SIGNATURE-----
This archive was generated by hypermail 2b25 : Wed May 31 2000 - 14:50:12 CDT