Subject: Re: ZAP/[patch] Martin's cursor position POW
From: Sam TH (sam@uchicago.edu)
Date: Thu Jan 25 2001 - 01:07:37 CST
On Wed, Jan 24, 2001 at 11:59:50PM -0500, John L. Clark wrote:
> Well guys, I think this may be the first ZAPPED POW of the new
> millenium. The url to the original POW announcement is
> http://www.abisource.com/mailinglists/abiword-dev/01/January/0198.html.
> Martin, would you be so kind as to check to see if this is what you
> wanted? Everyone else, let me know what you think, and how it works.
First off, I want to say that this is an excellent patch. It has nice
code, and it fixes the problem completely, although I'll wait for
Martin to confirm that.
I just have two nitpicky little minor concerns.
1) One minor coding style issue.
Instead of this:
if(lff) pActiveView = lff->getCurrentView();
else pActiveView = this;
we usually do this:
if(lff)
pActiveView = lff->getCurrentView();
else
pActiveView = this;
Not that one way is better than the other, but consistency is good.
2) Comments.
This is something new that I want to suggest. I think every new
function that we write (at a minimum, all the XP ones) should have at
least a short comment, preferably for Doxygen, explaining what it
does. If we follow this practice, our code will soon become much
clearer and easier to understand (keep your fingers crossed).
What do people think about that?
Again, great patch. Thanks
sam th
sam@uchicago.edu
http://www.abisource.com/~sam/
GnuPG Key:
http://www.abisource.com/~sam/key
This archive was generated by hypermail 2b25 : Thu Jan 25 2001 - 01:08:46 CST