I haven't had time to ~really~ look it over, but these are my
initial observations. Obviously they don't mean too much
anyway.
> Yes, the code is primitive
Of course there's no way Alex could accept this in any short
period of time.
Thanks for the patch. Your contributions are very good :>
But the format of your patch is very annoying :(
and makes review by anyone difficult.
The constant change of spacing and changes like
- if (storedLineMoves_[line].from != NULL_SQUARE) {
+ if (storedLineMoves_[line].from != NULL_SQUARE)
+ {
are a pain to sift through.
You have mixed up core changes like inlining ReadTwoBytes
with the bestgames feature change. These should be in separate patches, not
together. Large patches like this should have
documentation about wtf they do. Ditto the Storedline changes.
You've removed the button bar. Congratulations...
but this should be totally separate otherwise you're wasting
the time of everyone who reviews this patch.
Sorry for being so blunt, and perhaps i am also out of line.
> For the second bug ::tree::make checks the existence of a
> window .
> treeWin$baseNumber instead should check the variable
> ::treeWin$baseNumber
SCID's tree widgets are very poor in some regards.
The tree/bestlines (and probably, graph) should be in one
window. There is also some annoying problems with <Destroy>
and these widgets which need debugging (as you will have
noticed)
> I suppose that even the first bug is caused by a name
> mismatch (::
> createToplevel add .fdock to the window name)
>
> By the way, what's your opinion on the tree & filter
> behavior?
It seems ok. You use "all games" to (not) restrict tree stats
to involve games in the filter. In undocked mode the
bestgame treeview widget does not have an x-scrollbar,
which is a little unusual.
Thanks for illustrating how to bind context menus to treeview,
which i'd never bothered to sort out.
And the changes to how the game is loaded via bestgames
and "::game::Load $idx $ply" is good.
The bestgames treeview widget looks fine, but out of
place compared to the other, old, tree widgets.
I don't think your changes to the browser window are very good.
-------------------------
Is there ~much~ advantage inlining ReadTwoBytes etc ?
What do the changes to Storedline mean ?
Steve
------------------------------------------------------------------------------
Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)!
Finally, a world-class log management solution at an even better price-free!
Download using promo code Free_Logger_4_Dev2Dev. Offer expires
February 28th, so secure your free ArcSight Logger TODAY!
http://p.sf.net/sfu/arcsight-sfd2d
_______________________________________________
Scid-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/scid-users
