[haiku-bugs] Re: [Haiku] #11518: Implement BFont::Blocks

  • From: "axeld" <trac@xxxxxxxxxxxx>
  • Date: Tue, 02 May 2017 07:47:22 -0000

#11518: Implement BFont::Blocks
--------------------------------+------------------------------------------
   Reporter:  dsizzle           |      Owner:  nobody
       Type:  enhancement       |     Status:  assigned
   Priority:  normal            |  Milestone:  R1
  Component:  Kits/Interface    |    Version:  R1/Development
  Kit                           |
 Resolution:                    |   Keywords:  bfont, blocks, font, unicode
 Blocked By:                    |   Blocking:  3651
Has a Patch:  1                 |   Platform:  All
--------------------------------+------------------------------------------

Comment (by axeld):

 The constants should be added where they fit best, so the change is okay
 in this regard. The remote app_server works on the `DrawingInterface`
 level, it doesn't know anything about windows and such.

 Besides this, if we wanted a stable interface, we should set constants
 manually instead; an enumeration with automatic values is not really
 suited for this.

 The patch looks quite nice overall, but I have a few questions/remarks:
 1. What's the purpose of the unicode_block_range array? I'm wondering a
 bit as it uses the old unicode_block, but I don't quite see why. If there
 is something new, it should contain all the new unicode block ranges, too
 (those in the Supplementary Multilingual Plane and Supplementary
 Ideographic Plane), but I'm not sure this would have to be part of the
 public API, as it would be rarely needed by applications.
 1. ServerFont.cpp, line 616: it should break out of the loop.

 And finally, there are a few more minor coding style violations I'd like
 to point out:
 - Font.h line 129: stray leading space.
 - Font.cpp line 848, and 870: why not put the if clause on one line?
 - ServerFont.cpp: `FindBlockForCodepoint()` and `ParseFcMap()` should be
 static.
 - ServerFont.cpp line 496, and other places: use "& " style, not " &".
 - line 514: Operators go to the next line. Multi-line if statements are
 always surrounded by {}.
 - line 521: If an argument doesn't fit into the 80 character column limit,
 it's only indented a single tab relative to its origin line.
 - line 532: Operators go to the next line.

--
Ticket URL: <https://dev.haiku-os.org/ticket/11518#comment:33>
Haiku <https://dev.haiku-os.org>
The Haiku operating system.

Other related posts: