[haiku-development] Re: Final Set*UIColor Patch, Version 3e

  • From: looncraz <looncraz@xxxxxxxxxxx>
  • To: haiku-development@xxxxxxxxxxxxx
  • Date: Sun, 22 Nov 2015 18:20:55 -0600

On 11/22/2015 15:44, Stephan Aßmus wrote:

Hi,
Howdy!
Scrollbar in the browser window tells me I am still below 25% of the complete patch.

Thank you for looking at it! You should be close to done viewing the primary changes, then ;-)

It appears larger than it really is.

So far, I think the new API which got introduced is fine. Some remarks below. Because of that, and I think this is what Axel said as well, I tend to think it would be good to commit the patch, and change some of the implementation later, when time permits and if necessary. But the implementation should not impact the new API very much, since most application code does not deal with the new BColorMap.

Very much my line of reasoning. The public API should be clean, tidy, and the details have all been tucked away and can be changed any time in the future as, and if, the need arises.


* I would suggest "B_FOLLOW_LEFT_TOP" is better (more meaningful) than "B_FOLLOW_DEFAULT".

I debated that, and probably should have gone that route. If you want, I can make that change tonight quite easily.


* I don't like the changes to the textview stuff in HaikuDepot (the CharacterStyle[Data]) so much. My feeling is that the text view API should not be concerned with Interface Kit stuff (beyond graphics primitive types like rgb_color). And API dealing with "color_which" would be such a misplaced (in my view) "dependency". The solution as I see it, would be to implemented the inheritence support I planned for CharacterStyle. If HaikuDepot integrating with live color changes is an important concern, I could live with these parts being commited, and removed again later, once inheritence is implemented.

I didn't like it after I made it :p, but it solves the problem for the time being. If you're okay with it for now so it gets to fit with the system better, then that's great. I'd love to see what you have planned for that text system.


I hope to find more time to review the rest of the patch soon.

Thank you!



Other related posts:

  1. Home
  2. haiku-development
  3. Archive
  4. 11-2015