[Libreoffice] [PUSHED] Docuview code cleanup #2

[Michael Meeks]

Hi Matteo,

On Thu, 2011-11-24 at 02:04 +0100, Matteo Casalin wrote:
> * a revised patch, with a new ImplDrawSymbol;

	Lovely :-) it reads more nicely now, I pushed it & good work.

> * a picture with the drawings of all symbols, produced by both the old
>    and by the new routines. All symbols were drawn with different sides
>    of their target rectangle, and for each rectangle size 4 full
>    sequences of symbols are drawn:

	All looks good to me. I love the improved float icon too :-)

> Yeah, using polygons could reduce that code, but I just begun 
> contributing and I don't feel comfortable with such a big change, at 
> least for now.

	Heh :-)

>  Besides, I had a quick look at (rendering of) polygons 
> and it looks a little too complicated for such small symbols and, if you 
> take a look at the circles generated by the original ImplDrawSymbols 
> (which made use of polygons), you'll see that the results were not so 
> precise.

	Right; I saw you replaced that.

> No problem, I see that there's a lot of activity in the repository :)
> I'm planning to do some more cleanups in Docuview, I'll post them little 
> by little. Is this kind of activities appreciated or would bug-solving 
> be better ?

	Oh - well, everything gratefully received :-) whatever interests you
most is best I think. Of course, some of our bigger, nastier problems
are in the are of user-interface; 

>  I'm asking this because this task was chosen by chance, more 
> for training than for other reason.

	Sure; so - you could poke another easy-hack, or carry on cleaning this
up. There are really plenty of interesting tasks - it often helps to
have several on the queue :-)

	Of course, things that improve the UI are much appreciated by end
users, and there are a number of open tasks in this area that are not
easy hacks if you're interested (Christophe had a nice idea of making it
rather easier to add notes by clicking on the ruler IIRC that might be
fun) :-)

> Another question on preferred behaviour for future contributions: should 
> I have posted this new patch as a new mail, with an explicit [PATCH] 
> header in its subject?

	That's best practise yes, but luckily I was watching for this :-)

On Thu, 2011-11-24 at 12:22 +0100, Matteo Casalin wrote:
>     please also note that for SYMBOL_CHECKMARK I kept both normal and 
> mirrored versions of the symbol, as in the original routine, although 
> the comment
> 
> // #106953# never mirror checkmarks

	Heh :-)

> seems to mean that this is not desired. But I don't know if the 
> comment is obsolete, misleading (could it mean that the bug was that 
> checkmarks was not mirrored, as they should?) or correct (and in the 
> latter case implementation is wrong). As a beginner I really don't 
> know where to search for such a bug report and, honestly, I'm a bit 
> puzzled by bug identifiers (fdo#, i# or whatelse).

	Ah - so - this bug is to a dead & obsolete Sun/Oracle bug tracker that
rides no more: it'd be good (in fact) to come up with a nice regexp to
find them all across the code & remove them.

> I did some internet research about checkmarks in RTL, without results.
> Unless the original bug report is found, this issue should/could
> eventually be clarified with some RTL people.

	Sure - poke Lior Kaplan he's the man for RTL queries. I strongly
suspect the comment is right though - I don't think those guys want
backwards ticks in boxes ;-)

	Anyhow - thanks for the great work; looking forward to your next steps.

	All the best,

		Michael.

-- 
michael.meeks at suse.com  <><, Pseudo Engineer, itinerant idiot